feat: self-healing engine — rule failure detection + auto-patching#21
feat: self-healing engine — rule failure detection + auto-patching#21
Conversation
New: behavioral_extractor.py - 12-archetype sentence-level extraction for actionable lesson descriptions - Replaces word-diff summaries with imperative instructions - Template-based with upgrade path for LLM refinement New: correction_patterns table (meta_rules_storage.py) - Cross-session pattern tracking with batch upsert - Graduation candidate query for repeated patterns Changed: _core.py - brain.correct() uses behavioral_extractor as primary extraction path - Falls back to keyword templates New: test_pipeline_e2e.py (12 tests) - Pattern tracking, correction logging, injection formatting - Cloud-dependent tests skip on CI 1699 tests passing, 0 failures. Co-Authored-By: Gradata <noreply@gradata.ai>
Use getattr() instead of direct attribute access for spawn_queue to satisfy Pyright's type narrowing after hasattr() check. Co-Authored-By: Gradata <noreply@gradata.ai>
…LI --profile flag Introduces the Python hook system foundation: - _profiles.py: MINIMAL/STANDARD/STRICT profile tiers - _base.py: shared run_hook protocol with profile gating - _installer.py: 17-hook registry with generate/install/uninstall/status - Refactors claude_code.py to delegate to _installer - Adds --profile flag to `gradata hooks install` - 18 new tests, 1712 total passing Co-Authored-By: Gradata <noreply@gradata.ai>
…e, refactor auto_correct Phase 2 of hook port: wire the learning loop into Claude Code hooks. - auto_correct.py: add HOOK_META + run_hook entry point (keeps all existing logic) - inject_brain_rules.py: SessionStart hook that parses lessons.md and injects top-10 graduated rules as <brain-rules> XML - session_close.py: Stop hook that emits SESSION_END event and runs graduation sweep - 9 new tests covering parsing, scoring, injection, and session close Co-Authored-By: Gradata <noreply@gradata.ai>
…ment Port secret-scan.js to Python, add config file protection, and inject RULE-tier lessons before edits. All three are PreToolUse blocking hooks. 14 tests covering all paths. Co-Authored-By: Gradata <noreply@gradata.ai>
…ICT profiles) STANDARD: agent_precontext, agent_graduation, tool_failure_emit, tool_finding_capture, context_inject, config_validate, pre_compact. STRICT: duplicate_guard, brain_maintain, session_persist, implicit_feedback. 34 tests covering all hooks (happy path + no-op). Co-Authored-By: Gradata <noreply@gradata.ai>
…ib SequenceMatcher - Extract resolve_brain_dir() and extract_message() into _base.py, replacing 11 duplicate inline implementations - Replace inline RULE_RE parsers in inject_brain_rules, rule_enforcement, agent_precontext with self_improvement.parse_lessons() - Delete dead FAILURE_PATTERNS list from tool_finding_capture.py - Replace custom _levenshtein() with difflib.SequenceMatcher in duplicate_guard.py - Net reduction: ~127 lines removed across 14 hook files Co-Authored-By: Gradata <noreply@gradata.ai>
…nsolidate brain dir - Remove redundant tool_name check in secret_scan (matcher handles it) - Fix overly broad except (json.JSONDecodeError, Exception) to except Exception - Remove dead _HOOK_NAME, _HOOK_CONFIG, _load_settings, _save_settings from claude_code.py - Use resolve_brain_dir() in claude_code.py capture_correction() - Remove unused os/Path imports from claude_code.py - Remove unnecessary comment in _installer.py Co-Authored-By: Gradata <noreply@gradata.ai>
…or, nitpick CRITICAL: auto_correct main() accepts data dict, emit() uses ctx not brain_dir (agent_graduation, implicit_feedback, tool_failure_emit, session_close), EventType replaced with string, graduation_sweep replaced with graduate(), installer matcher moved to group level. MAJOR: secret_scan scans MultiEdit edits, meta_rules_storage conn leak fixed, config_validate validates nested hook entries, duplicate_guard resolves paths, pre_compact/tool_finding_capture use per-user temp dirs, session_persist includes untracked files. MINOR: context_inject separator in budget, behavioral_extractor module-level import + boundary-aware matching + relaxed actionable check, installer handles corrupt JSON + uses logging. NITPICK: l->lesson rename, get_brain return type, debug logging for suppressed exceptions. 1774 tests pass, 17 skipped. Co-Authored-By: Gradata <noreply@gradata.ai>
…pick Critical: auto_correct main() signature, EventType→string, emit ctx param, brain_maintain BrainContext threading, graduation_sweep→inline graduation Major: MultiEdit secret scanning, SQLite conn leak, config_validate nesting, duplicate_guard path normalization, per-user temp dirs, session_persist untracked files Minor: corrupt settings.json handling, context budget, behavioral_extractor boundaries Nitpick: l→lesson renames, return type annotation, debug logging, logging vs print Co-Authored-By: Gradata <noreply@gradata.ai>
Remove dead state_str assignment in inject_brain_rules._score(), add debug logging for implicit_feedback inner except block, and clamp PATTERN_SEVERITY_WEIGHTS values to [0,1] range. All other 28 findings were already resolved in prior commits. Co-Authored-By: Gradata <noreply@gradata.ai>
- inject_brain_rules: remove dead identical-branch conditional - implicit_feedback: add debug logging for emit failures - meta_rules_storage: clamp PATTERN_SEVERITY_WEIGHTS to [0,1] - auto_correct: remove dead file_path fetch, direct dict processing - _base: improve return type annotation, remove redundant pass - secret_scan: redact secret previews, add MultiEdit support - config_validate: flexible Python executable matching - duplicate_guard: add debug logging for scan errors - brain_maintain: remove unused brain_dir param - pre_compact: per-session snapshot filenames, module-level imports - behavioral_extractor: strict zip, expanded imperative starters - rule_canary: future annotations, logging, off-by-one fix - rule_integrity: extract shared regex, add confidence clamping - session_persist: explicit check=False, dict.fromkeys dedup - tool_finding_capture: exact basename matching Co-Authored-By: Gradata <noreply@gradata.ai>
…ondition rule_canary.py: use context managers for all SQLite connections to prevent leaks on exception. tool_finding_capture.py: atomic temp+rename writes to prevent corruption from concurrent hook processes. secret_scan: simplify loop. Co-Authored-By: Gradata <noreply@gradata.ai>
Merged github/main into feat/sdk-hook-port. Resolved conflicts in _core.py, behavioral_extractor.py, meta_rules_storage.py, test_pipeline_e2e.py. Kept main's meta-rule logic + feature branch's CodeRabbit fixes. 1775 tests pass. Co-Authored-By: Gradata <noreply@gradata.ai>
Co-Authored-By: Gradata <noreply@gradata.ai>
Co-Authored-By: Gradata <noreply@gradata.ai>
Co-Authored-By: Gradata <noreply@gradata.ai>
Co-Authored-By: Gradata <noreply@gradata.ai>
Co-Authored-By: Gradata <noreply@gradata.ai>
Co-Authored-By: Gradata <noreply@gradata.ai>
…hase 2 prep) Co-Authored-By: Gradata <noreply@gradata.ai>
… flow Co-Authored-By: Gradata <noreply@gradata.ai>
Remove unused _log, _ACTIVE_STATES, and logging import from self_healing.py. Drop unused category param from _generate_deterministic_patch. Reuse brain._load_lessons() in _core.py instead of manual parse. Extract deeply nested nudging logic in implicit_feedback.py into _check_nudges() helper. Co-Authored-By: Gradata <noreply@gradata.ai>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a self-healing pipeline (detection, deterministic patching, retroactive gating, nudging, scope narrowing), rule integrity signing and canary lifecycle, a profile-driven hook framework with an installer and ~20 new Python hooks (replacing legacy JS hooks), CI and MkDocs configs, extensive tests, and many EOF/newline and small refactors across the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant Tool as Tool / Claude
participant HookRunner as Hook Runner
participant Brain as Brain
participant Lessons as Lessons File
participant Events as Event Bus
participant DB as Canary/Sign DB
Tool->>HookRunner: emits correction or tool output
HookRunner->>Brain: call brain.correct(draft, final)
activate Brain
Brain->>Lessons: load/parse lessons.md
Lessons-->>Brain: parsed lessons
Brain->>Brain: detect_rule_failure(category, description)
alt rule failure detected
Brain->>Events: emit RULE_FAILURE (tags include self_healing)
Brain->>Lessons: apply_patch via patch_rule(...)
Lessons-->>Brain: lesson file persisted
Brain->>Events: emit RULE_PATCHED
Brain->>DB: check/promote/rollback canary entries
end
Brain-->>HookRunner: return correction outcome
deactivate Brain
sequenceDiagram
participant CLI as User CLI
participant Installer as Hook Installer
participant File as ~/.claude/settings.json
participant Profile as Profile manager
CLI->>Installer: install(profile)
Installer->>Profile: evaluate registry for profile
Installer->>File: load existing settings
Installer->>Installer: generate_settings(profile)
Installer->>File: write merged settings
File-->>Installer: persisted
Installer-->>CLI: install complete
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 29
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/gradata/hooks/claude_code.py (1)
55-55:⚠️ Potential issue | 🟡 MinorDead code: unused expression result.
This statement retrieves
tool_outputbut discards the result. Either remove it or assign it if intended for future use.Proposed fix
- payload.get("tool_output", "") + # tool_output = payload.get("tool_output", "") # Uncomment if neededOr simply remove the line if not needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/hooks/claude_code.py` at line 55, The expression payload.get("tool_output", "") is a dead/unassigned expression; either remove this line or assign its result to a variable for later use (e.g., tool_output = payload.get("tool_output", "")) wherever payload is being processed in this module (referencing the payload variable in src/gradata/hooks/claude_code.py) so the value is not discarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/sdk-ci.yml:
- Around line 32-35: Replace the non-deterministic version pin in the GitHub
Actions step that installs uv (the step named "Install uv" which uses
astral-sh/setup-uv@v4) so it no longer uses version: "latest"; update the
with.version value to a specific release (e.g., "0.11.6") or a stable semver
range (e.g., "0.11.x") to ensure reproducible CI runs.
In `@sdk/mkdocs.yml`:
- Around line 30-56: The mkdocs navigation references many missing documentation
pages so the site build will fail; create the docs directory and add the exact
markdown files referenced by the nav entries (e.g., index.md;
getting-started/install.md, getting-started/quickstart.md,
getting-started/concepts.md; cloud/overview.md, cloud/quickstart.md,
cloud/api.md, cloud/marketplace.md; guides/training.md,
guides/custom-domains.md, guides/mcp.md, guides/agent-graduation.md;
api/brain.md, api/cli.md, api/patterns.md; architecture/overview.md,
architecture/patterns.md, architecture/enhancements.md, architecture/brain.md,
architecture/agent-graduation.md) with at least a frontmatter/title and
placeholder content so mkdocs can build, ensuring file names and directory
structure match the nav keys (e.g., "Getting Started" => getting-started/*,
"Gradata Cloud" => cloud/*, etc.).
- Around line 58-67: The mkdocstrings handler in mkdocs config currently points
to the wrong source (paths: [src]) causing docs/CI to look for sdk/src; update
the mkdocstrings paths setting (under the mkdocstrings -> handlers -> python ->
paths key) to paths: [../src] so it correctly references the repository Python
package (or alternatively adjust SDK layout), and ensure this matches how the
SDK CI references src/gradata so both docs rendering and CI use the same ../src
location.
In `@src/gradata/brain.py`:
- Around line 346-356: The patch routine currently uses _find_lessons_path()
which may return external fallback files; change patch_rule to only allow edits
to the brain-local lessons file by resolving the canonical local path (e.g.
local = self.path.joinpath("lessons.md") or its resolved equivalent) and
verifying lessons_path is the same file or is_relative_to(self.path) before
proceeding; if the located lessons_path is external, return {"patched": False,
"error": "read_only: external lessons file"} and avoid calling
write_lessons_safe/format_lessons/apply_patch on external files (symbols:
_find_lessons_path, patch_rule, write_lessons_safe, format_lessons,
apply_patch).
In `@src/gradata/enhancements/meta_rules_storage.py`:
- Around line 447-450: The code opens sqlite3 connection (conn) and immediately
queries the correction_patterns table which can raise sqlite3.OperationalError
on a fresh DB; before running the SELECT, either (a) check for table existence
with a quick query like "SELECT name FROM sqlite_master WHERE type='table' AND
name='correction_patterns'" and skip/return empty if missing, or (b) wrap the
execute/select of correction_patterns in a try/except catching
sqlite3.OperationalError and returning an empty result; apply the same change to
the other block that queries correction_patterns (the block around lines
487-490) so read-only callers don't fail on an unbootstrapped db_path with no
tables.
In `@src/gradata/enhancements/rule_canary.py`:
- Around line 251-258: The emit call inside the promote-to-active block (the
emit("CANARY_PROMOTED", "rule_canary:promote_to_active", ... ) call in
rule_canary.py) is missing the BrainContext parameter; update this emit
invocation to pass the current BrainContext (as done in rollback_rule) so path
resolution is consistent—i.e., supply ctx (or the appropriate BrainContext
variable available in this scope) as the ctx argument and keep the existing
event name, action, payload, and tags.
- Around line 80-87: The brittle five-level parent traversal using
Path(__file__).resolve().parent.parent.parent.parent.parent to set scripts_dir
should be replaced with a robust resolution: either locate the installed "brain"
package via importlib (e.g., importlib.util.find_spec or
importlib.resources.files) and derive scripts_dir from that package location, or
iterate up the Path(__file__).resolve().parents in a small loop (check each
parent up to a sensible maximum) and test for "brain/system.db" rather than
hardcoding five parents; update the code that sets scripts_dir and p (the Path
for "brain/system.db") accordingly so existence checks remain the same but
resolution is resilient to package layout changes.
- Around line 148-158: The current SELECT uses a fragile LIKE on data_json;
change the query in the try block that builds corr_row to use JSON extraction
for the rule_category field instead (use json_extract(data_json,
'$.<fieldName>') = ? with the rule_category param and the same CAST(session AS
INTEGER) >= ? bound value) so you match the JSON key exactly; keep using
corr_row["cnt"] to set correction_count and preserve the existing except
fallback, but replace the LIKE pattern and parameter with an exact json_extract
comparison referencing the same rule_category variable and start_session.
- Around line 214-227: The emit call in rule_canary.py is missing the ctx
parameter required for proper path resolution and multi-brain support; update
the emit invocation in rollback_rule (the call that currently emits
"RULE_ROLLBACK", "rule_canary:rollback_rule", {...}) to pass the current
BrainContext (ctx) as the ctx argument (i.e., emit(..., ctx=ctx)) so it uses the
proper context rather than global state; ensure you import or reference the
existing BrainContext/ctx variable used elsewhere in the surrounding function if
necessary.
In `@src/gradata/enhancements/rule_integrity.py`:
- Around line 209-222: The _ensure_table function should use a context manager
for the SQLite connection: replace the explicit conn = sqlite3.connect(...) /
try / finally / conn.close() pattern with using "with
sqlite3.connect(str(db_path)) as conn:" so the connection is closed
automatically and commit is performed on successful exit; keep the CREATE TABLE
IF NOT EXISTS SQL and you can omit an explicit conn.close() (explicit
conn.commit() is optional inside the with-block but not required).
- Around line 159-169: The loop in extract_rule_signatures currently overwrites
signatures[category] with the last matched rule per category (using sign_rule
and _LESSON_PATTERN), which silently drops earlier rules; either change
signatures from dict[str, str] to dict[str, list[str]] (append each sig to
signatures[category]) and update the function return type/docstring to reflect
multiple signatures per category, or retain the single-signature design but add
a clear warning log whenever an existing signatures[category] would be
overwritten (include category and both old/new sigs) so overwrites are explicit;
locate this logic in extract_rule_signatures, where signatures is assigned and
where sign_rule(...) is called.
In `@src/gradata/enhancements/self_healing.py`:
- Around line 79-96: The comparison in apply_patch currently uses an exact
string match on lesson.description against old_description, which can fail with
whitespace differences; normalize both strings before comparing (e.g., collapse
consecutive whitespace and trim using a function like normalize = lambda s: "
".join(s.split())) and use normalize(lesson.description) ==
normalize(old_description) to find the lesson, then assign lesson.description =
new_description (leave new_description unchanged) and return the lesson as
before.
In `@src/gradata/hooks/_base.py`:
- Around line 84-97: Extract the hardcoded event tuple ("SessionStart", "Stop",
"PreCompact") into a descriptive module-level constant (e.g., EVENT_WHITELIST or
EVENTS_ALLOW_EMPTY_DATA) and update run_hook to reference that constant instead
of the inline list; ensure the constant is defined at top-level near other
module constants and include a short comment explaining its purpose so future
event additions only need to update the single constant used in the
meta.get("event") membership check inside run_hook.
In `@src/gradata/hooks/_installer.py`:
- Around line 14-16: The import order is inconsistent: _log is initialized
before other module imports; move the import of Profile (from
gradata.hooks._profiles) up into the top import block so all imports appear
before logger setup (ensure _log = logging.getLogger(__name__) remains after
imports), updating the symbols _log and Profile in
src/gradata/hooks/_installer.py accordingly.
In `@src/gradata/hooks/agent_graduation.py`:
- Around line 49-50: Replace the silent except block that currently reads
"except Exception: pass" with a debug log that captures the exception; e.g.,
change to "except Exception as e:" and call the module's logger (or
logging.getLogger(__name__)) at debug level including the exception information
(use logger.debug('agent_graduation hook failed', exc_info=e) or
logger.debug('agent_graduation hook failed: %s', e, exc_info=e)) so the hook
still swallows errors but records them for debugging in
src/gradata/hooks/agent_graduation.py (where the except block lives).
In `@src/gradata/hooks/agent_precontext.py`:
- Around line 47-58: The _relevance_score function currently gives a +0.2 bonus
only when lesson.state.name == "RULE"; modify _relevance_score to also award a
smaller bonus (e.g., +0.1) when lesson.state.name == "PATTERN" so PATTERN
lessons rank above lower tiers but below RULE; implement this by adding a
conditional branch checking lesson.state.name == "PATTERN" (or converting to
.lower() for case-insensitivity) and adding 0.1 to score where appropriate,
keeping the existing RULE branch unchanged.
In `@src/gradata/hooks/brain_maintain.py`:
- Around line 16-39: The _rebuild_fts function currently swallows all exceptions
silently; update its exception handlers to log suppressed exceptions at debug
level (including exception details/traceback) instead of using bare pass so
debugging is possible (mirror the pattern used in implicit_feedback.py).
Specifically, in _rebuild_fts wrap the outer try/except and the inner per-file
except Exception blocks to call the module or ctx logger (or a logger obtained
via logging.getLogger) and emit a debug-level message with the exception info
when fts_index or file reads fail, while preserving the silent/fail-safe
behavior for callers.
In `@src/gradata/hooks/config_protection.py`:
- Around line 15-25: PROTECTED_FILES currently includes "pyproject.toml", which
blocks all edits; remove "pyproject.toml" from the PROTECTED_FILES set (in
src/gradata/hooks/config_protection.py) so general project metadata and
dependency changes are allowed, or alternatively keep it but change the
protection logic that checks PROTECTED_FILES to parse the modified
pyproject.toml and deny only edits to tool-specific sections (e.g., [tool.ruff],
[tool.black], [tool.isort], etc.) by inspecting the diffed TOML keys before
rejecting the write.
In `@src/gradata/hooks/config_validate.py`:
- Around line 59-71: The repeated import of gradata.hooks and lookup of
hooks_pkg.__file__ inside the nested loops is inefficient; move the import and
resolution of hooks_dir outside the loops (once) — attempt a single import of
gradata.hooks into hooks_pkg, compute hooks_dir =
Path(hooks_pkg.__file__).parent (handle the import exception once and set a flag
to skip file checks if it fails), then inside the loop keep computing
module_name and module_path = hooks_dir / f"{module_name}.py" and append the
same warnings referencing event_name, i, j only if hooks_dir is available.
In `@src/gradata/hooks/duplicate_guard.py`:
- Around line 75-78: The _in_watched_dir function uses a substring check against
WATCHED_DIRS which can yield false positives (e.g. "/.../gradata_backup"
matching "src/gradata/"); change the logic to perform path-component-aware
matching: normalize file_path to a pathlib.Path (e.g.,
Path(file_path).as_posix() or Path(file_path).parts) and for each watched entry
(also normalized) check for exact path component boundaries (for example compare
Path(file_path).parts to the sequence of parts from Path(watched).parts or use
Path(file_path).resolve().match or startswith on the resolved path with a
normalized watched path + os.sep) so only true directory matches are accepted;
update _in_watched_dir to use pathlib and WATCHED_DIRS normalized parts to avoid
substring matches.
In `@src/gradata/hooks/implicit_feedback.py`:
- Around line 113-118: The imports currently inside the for cat in
categories_seen loop (from datetime import date as _date; from gradata._types
import Lesson, LessonState; from gradata.enhancements.self_improvement import
format_lessons, parse_lessons, INITIAL_CONFIDENCE; from gradata._db import
write_lessons_safe) should be moved to the top of the function (or module) that
contains the loop to avoid repeated import overhead; locate the loop named "for
cat in categories_seen" and remove those in-loop import statements, adding them
once at the start of the surrounding function or module so the code references
the same symbols ( _date, Lesson, LessonState, format_lessons, parse_lessons,
INITIAL_CONFIDENCE, write_lessons_safe) without re-importing each iteration.
In `@src/gradata/hooks/session_close.py`:
- Around line 23-35: In _run_graduation replace the direct
lessons_path.write_text(...) call with the existing locked writer
write_lessons_safe() to avoid unlocking races: after computing formatted =
format_lessons(active) call write_lessons_safe(brain_dir, formatted) (or the
module’s write_lessons_safe API) instead of Path.write_text; ensure you import
write_lessons_safe from gradata.enhancements.self_improvement alongside
parse_lessons/graduate/format_lessons and preserve UTF‑8 semantics and existing
exception handling.
In `@src/gradata/hooks/tool_failure_emit.py`:
- Around line 17-26: The ERROR_PATTERNS list contains a case-sensitive pattern
re.compile(r"\bException\b") which misses lowercase "exception" entries; modify
that element in ERROR_PATTERNS to use the re.I flag (i.e., compile
r"\bException\b" with re.I) so it matches case-insensitively, keeping the rest
of the list unchanged.
In `@src/gradata/hooks/tool_finding_capture.py`:
- Around line 116-121: The current loop in tool_finding_capture.py clears all
findings by calling _save_findings([]) as soon as any file basename matches;
change this to remove only the matched finding(s) from the findings list and
persist the reduced list via _save_findings(updated_findings). Specifically,
inside the nested loops where you iterate over findings and each
finding.get("files", []), detect matches using Path(f).name == file_basename,
collect the matching finding ids or indices (or filter by identity), build a new
list excluding those matched entries, then call _save_findings(new_findings) and
return the same result string; keep the existing variables (findings,
file_basename, Path, _save_findings) to locate and implement the change.
In `@tests/test_hooks_base.py`:
- Around line 24-27: The test test_get_profile_default contains a redundant
os.environ.pop("GRADATA_HOOK_PROFILE", None) after using patch.dict(os.environ,
{}, clear=True); remove the pop call so the test simply clears the environment
with patch.dict and asserts get_profile() == Profile.STANDARD, keeping
references to test_get_profile_default, patch.dict, get_profile, and
Profile.STANDARD to locate the change.
In `@tests/test_hooks_intelligence.py`:
- Around line 31-37: The test uses a hardcoded POSIX nonexistent path when
calling precontext_main in test_agent_precontext_no_brain; change it to use the
pytest tmp_path fixture (e.g., str(tmp_path / "missing")) and patch.dict with
that string so the path is platform-independent; apply the same replacement for
the other tests referenced (lines around 76-83, 262-265, 346-349, 377-380) that
currently hardcode "/nonexistent/..." paths and ensure test functions accept the
tmp_path fixture where needed.
In `@tests/test_hooks_learning.py`:
- Around line 22-24: The test does a direct float equality check on
rules_and_patterns[0].confidence which is fragile; change the assertion to use
pytest.approx for robust comparison (e.g., assert
rules_and_patterns[0].confidence == pytest.approx(0.92)) and ensure pytest is
available in the test namespace (import pytest if not already present) so that
the confidence comparison in tests/test_hooks_learning.py uses pytest.approx
rather than ==.
In `@tests/test_hooks_safety.py`:
- Around line 13-67: The test contains an intentional dummy secret in
test_secret_scan_blocks_openai_key that triggers a static-analysis false
positive; to fix, mark that test value as test-only so the scanner ignores it by
adding an inline suppression/comment on the content string (e.g., append a
recognized ignore token like "# nosec" or a project-specific Betterleaks
suppression comment and a short note "dummy test key"), or alternatively
consolidate cases by parametrizing the tests into a single
test_secret_scan_detection that iterates (content, should_block) and calls
scan_main, as shown in the reviewer example; update the test names
(test_secret_scan_blocks_openai_key or test_secret_scan_detection) and the
scan_main usage accordingly so the intent and suppression are clear.
---
Outside diff comments:
In `@src/gradata/hooks/claude_code.py`:
- Line 55: The expression payload.get("tool_output", "") is a dead/unassigned
expression; either remove this line or assign its result to a variable for later
use (e.g., tool_output = payload.get("tool_output", "")) wherever payload is
being processed in this module (referencing the payload variable in
src/gradata/hooks/claude_code.py) so the value is not discarded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 126467f9-4357-4275-8387-e97215793678
⛔ Files ignored due to path filters (3)
375x812-desktop.pngis excluded by!**/*.png375x812-mobile.pngis excluded by!**/*.png375x812-tablet.pngis excluded by!**/*.png
📒 Files selected for processing (40)
.github/workflows/sdk-ci.ymlsdk/mkdocs.ymlsrc/gradata/_core.pysrc/gradata/brain.pysrc/gradata/cli.pysrc/gradata/enhancements/behavioral_extractor.pysrc/gradata/enhancements/meta_rules_storage.pysrc/gradata/enhancements/rule_canary.pysrc/gradata/enhancements/rule_integrity.pysrc/gradata/enhancements/self_healing.pysrc/gradata/hooks/_base.pysrc/gradata/hooks/_installer.pysrc/gradata/hooks/_profiles.pysrc/gradata/hooks/agent_graduation.pysrc/gradata/hooks/agent_precontext.pysrc/gradata/hooks/auto_correct.pysrc/gradata/hooks/brain_maintain.pysrc/gradata/hooks/claude_code.pysrc/gradata/hooks/config_protection.pysrc/gradata/hooks/config_validate.pysrc/gradata/hooks/context_inject.pysrc/gradata/hooks/duplicate_guard.pysrc/gradata/hooks/implicit_feedback.pysrc/gradata/hooks/inject-brain-rules.jssrc/gradata/hooks/inject_brain_rules.pysrc/gradata/hooks/pre_compact.pysrc/gradata/hooks/rule_enforcement.pysrc/gradata/hooks/secret_scan.pysrc/gradata/hooks/session-history-sync.jssrc/gradata/hooks/session_close.pysrc/gradata/hooks/session_persist.pysrc/gradata/hooks/tool-finding-capture.jssrc/gradata/hooks/tool_failure_emit.pysrc/gradata/hooks/tool_finding_capture.pytests/test_hooks_base.pytests/test_hooks_intelligence.pytests/test_hooks_learning.pytests/test_hooks_safety.pytests/test_pipeline_e2e.pytests/test_self_healing.py
💤 Files with no reviewable changes (3)
- src/gradata/hooks/session-history-sync.js
- src/gradata/hooks/inject-brain-rules.js
- src/gradata/hooks/tool-finding-capture.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (3)
tests/**
⚙️ CodeRabbit configuration file
tests/**: Test files. Verify: no hardcoded paths, assertions check specific values not just truthiness,
parametrized tests preferred for boundary conditions, floating point comparisons use pytest.approx.
Files:
tests/test_pipeline_e2e.pytests/test_hooks_learning.pytests/test_hooks_safety.pytests/test_hooks_base.pytests/test_hooks_intelligence.pytests/test_self_healing.py
src/gradata/**/*.py
⚙️ CodeRabbit configuration file
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required), no print()
statements (use logging), all functions accepting BrainContext where DB access occurs, no hardcoded paths. Severity
scoring must clamp to [0,1]. Confidence values must be in [0.0, 1.0].
Files:
src/gradata/cli.pysrc/gradata/hooks/pre_compact.pysrc/gradata/_core.pysrc/gradata/brain.pysrc/gradata/enhancements/meta_rules_storage.pysrc/gradata/hooks/session_close.pysrc/gradata/hooks/auto_correct.pysrc/gradata/hooks/session_persist.pysrc/gradata/enhancements/behavioral_extractor.pysrc/gradata/hooks/brain_maintain.pysrc/gradata/hooks/rule_enforcement.pysrc/gradata/hooks/inject_brain_rules.pysrc/gradata/hooks/tool_failure_emit.pysrc/gradata/hooks/implicit_feedback.pysrc/gradata/hooks/agent_graduation.pysrc/gradata/hooks/config_protection.pysrc/gradata/hooks/config_validate.pysrc/gradata/hooks/secret_scan.pysrc/gradata/hooks/agent_precontext.pysrc/gradata/hooks/context_inject.pysrc/gradata/hooks/tool_finding_capture.pysrc/gradata/hooks/_base.pysrc/gradata/hooks/_installer.pysrc/gradata/hooks/claude_code.pysrc/gradata/enhancements/self_healing.pysrc/gradata/hooks/duplicate_guard.pysrc/gradata/hooks/_profiles.pysrc/gradata/enhancements/rule_integrity.pysrc/gradata/enhancements/rule_canary.py
src/gradata/hooks/**
⚙️ CodeRabbit configuration file
src/gradata/hooks/**: JavaScript hooks for Claude Code integration. Check for: no shell injection (no execSync with user
input), temp files must use per-user subdirectory, HTTP calls must have timeouts, errors must be silent (never block
the tool chain).
Files:
src/gradata/hooks/pre_compact.pysrc/gradata/hooks/session_close.pysrc/gradata/hooks/auto_correct.pysrc/gradata/hooks/session_persist.pysrc/gradata/hooks/brain_maintain.pysrc/gradata/hooks/rule_enforcement.pysrc/gradata/hooks/inject_brain_rules.pysrc/gradata/hooks/tool_failure_emit.pysrc/gradata/hooks/implicit_feedback.pysrc/gradata/hooks/agent_graduation.pysrc/gradata/hooks/config_protection.pysrc/gradata/hooks/config_validate.pysrc/gradata/hooks/secret_scan.pysrc/gradata/hooks/agent_precontext.pysrc/gradata/hooks/context_inject.pysrc/gradata/hooks/tool_finding_capture.pysrc/gradata/hooks/_base.pysrc/gradata/hooks/_installer.pysrc/gradata/hooks/claude_code.pysrc/gradata/hooks/duplicate_guard.pysrc/gradata/hooks/_profiles.py
🪛 Betterleaks (1.1.1)
tests/test_hooks_safety.py
[high] 16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (50)
tests/test_pipeline_e2e.py (1)
304-304: LGTM — trailing newline added.The change correctly adds a trailing newline. The
>= 3.0comparison is acceptable here sincepytest.approxis primarily intended for equality comparisons where floating-point precision matters, not for threshold checks..github/workflows/sdk-ci.yml (1)
55-61: LGTM — test suite split is intentional.Separating
test_spec_compliance.pyfrom the main test run is a valid pattern when spec compliance tests have different characteristics (e.g., longer runtime, external dependencies, or different failure semantics).src/gradata/enhancements/rule_integrity.py (1)
90-106: LGTM — signing implementation is secure.The implementation correctly:
- Clamps confidence to
[0.0, 1.0]per coding guidelines- Uses deterministic JSON canonicalization with
sort_keys=True- Returns empty string when no key is configured (unsigned mode)
src/gradata/hooks/_profiles.py (1)
1-10: LGTM — clean profile enum.The
IntEnumchoice enables natural comparison (Profile.STANDARD >= Profile.MINIMAL) for profile-based hook gating. The tier naming is clear and the values are appropriately ordered.src/gradata/cli.py (2)
443-443: LGTM — profile parameter correctly passed.The
getattr(args, "profile", "standard")pattern safely handles cases where the argument might not be present, with an appropriate default.
564-565: LGTM — CLI argument aligns with backend.The choices
["minimal", "standard", "strict"]match the mapping keys in_installer.py:generate_settings(), ensuring valid profile values are passed through.src/gradata/_core.py (1)
380-401: LGTM — self-healing integration is correct.The implementation:
- Properly catches exceptions to avoid blocking corrections
- Enriches failure data with
correction_event_idandcorrection_severityfor audit trail- Uses debug-level logging for failures (appropriate for non-critical path)
- Emits
RULE_FAILUREevent with proper tags for downstream processingOne minor optimization:
brain._load_lessons()is called unconditionally. If this becomes a hot path, consider lazy loading or caching.src/gradata/hooks/agent_graduation.py (1)
24-51: LGTM — hook implementation follows conventions.The hook correctly:
- Resolves brain directory before processing
- Creates
BrainContextfor proper path resolution- Handles both string and dict
tool_outputtypes- Suppresses exceptions to avoid blocking the tool chain (per coding guidelines)
src/gradata/hooks/rule_enforcement.py (1)
1-52: LGTM!The hook implementation is clean and follows the established patterns:
- Graceful import fallback for
parse_lessons- Proper early returns when brain directory or lessons file is missing
- Correct filtering for RULE-tier lessons using
lesson.state.name == "RULE"- Description truncation at 120 chars with ellipsis indicator
- Respects
MAX_REMINDERSlimittests/test_hooks_learning.py (2)
32-42: LGTM!The
_scorecomparison tests correctly use inequality operators which don't requirepytest.approx. The test cases properly verify the scoring priority: RULE > PATTERN at equal confidence, and higher confidence wins within the same state.
44-91: LGTM!The integration tests properly:
- Use
tmp_pathfixture for isolation- Patch environment variables appropriately
- Assert specific content in results (not just truthiness)
- Cover edge cases: missing brain dir, missing lessons file, and session close behavior
src/gradata/enhancements/behavioral_extractor.py (4)
34-39: LGTM!Good defensive programming pattern—moving the import to module level with a fallback regex ensures the module works even if the edit_classifier import fails. The fallback regex maintains the same pattern for factual content detection.
205-206: LGTM!Adding
strict=Truetozip()calls is a good safety improvement. It will raiseValueErrorif the iterables have different lengths, catching potential bugs wheredraft_sentsanddraft_sent_sets(orfinal_*equivalents) could get out of sync.Also applies to: 252-253, 290-291
209-223: LGTM!Precomputing
draft_lowerandfinal_loweronce and using word-boundary regex (\b) for hedge/constraint detection is a cleaner and more accurate approach than repeatedinsubstring checks—it avoids false positives from partial word matches.
423-434: LGTM!The expanded
_is_actionablegate now accepts imperative-looking instructions that start with an uppercase character and have at least 3 words. This accommodates user-provided instructions from thePREFIX_INSTRUCTIONarchetype that may not start with a recognized imperative verb.src/gradata/hooks/duplicate_guard.py (1)
80-127: LGTM!The main function has good defensive design:
- Resolves to absolute path to prevent path traversal (line 88)
- Only guards new files, allows overwrites (line 94-95)
- Graceful fallback on any exception (lines 125-126)
- Clear blocking message with actionable guidance (lines 119-123)
src/gradata/hooks/brain_maintain.py (1)
51-64: LGTM!The main function correctly:
- Uses
BrainContext.from_brain_dir()for proper context initialization- Passes
ctxto both_rebuild_ftsand_generate_manifestfor DB access- Returns
Noneconsistently (hook doesn't produce output)- Suppresses exceptions to avoid blocking the tool chain
src/gradata/hooks/tool_failure_emit.py (1)
59-91: LGTM!The main function is well-designed:
- Handles both string and dict outputs (lines 62-63)
- Uses
BrainContext.from_brain_dir()for proper DB access- Truncates previews to reasonable lengths (command: 200, output: 300)
- Limits signals to 5 to prevent event bloat
- Silent error handling appropriate for hooks
tests/test_hooks_safety.py (1)
99-143: LGTM!The rule_enforcement tests properly verify:
- RULE lessons are injected while PATTERN/INSTINCT are excluded (lines 110-114)
- No output when no RULE lessons exist (lines 117-122)
- Long descriptions are truncated with "..." (lines 125-132)
- Missing brain directory returns None (lines 135-143)
src/gradata/hooks/implicit_feedback.py (2)
139-181: LGTM!The main function has good structure:
- Early returns for missing/short messages (lines 142-143)
- Proper exception handling with debug logging (lines 167-168, 174-175, 179-180)
- Uses
BrainContext.from_brain_dir()for event emission- Returns structured feedback result
72-91: No changes needed. The use of_load_lessons()and_find_lessons_path()is a deliberate system-wide pattern used throughout the codebase, including in core modules (daemon.py, _core.py, brain_inspection.py), tests, and other hooks (auto_correct.py). These private methods function as internal APIs that multiple legitimate parts of the system depend on. Flagging this as a problem in only this file misses the broader architectural context.> Likely an incorrect or invalid review comment.src/gradata/hooks/secret_scan.py (1)
1-75: LGTM!Well-structured secret detection hook with comprehensive pattern coverage. The silent failure mode (returning
None) correctly follows the "never block the tool chain" principle for hooks. The redaction approach (***REDACTED***) appropriately avoids leaking secrets in the output.src/gradata/hooks/claude_code.py (1)
18-33: LGTM!Clean delegation to the centralized
_installermodule. The lazy imports inside each function prevent circular dependencies and keep the module lightweight.src/gradata/hooks/agent_precontext.py (1)
1-95: LGTM overall!Well-structured hook with sensible agent type inference and relevance scoring. The fallback chain for
parse_lessonsimport and silent exception handling correctly follow hook conventions.tests/test_hooks_base.py (1)
1-121: LGTM!Comprehensive test coverage for the hook foundation modules. Tests verify profile gating, I/O helpers, exception suppression, and installer behavior across profile tiers.
src/gradata/hooks/context_inject.py (1)
1-63: LGTM!Clean implementation with appropriate guards (minimum message length, command prefix filtering, length limiting). The fallback chain for result text extraction (
text→content→str(r)) provides good resilience.src/gradata/hooks/tool_finding_capture.py (1)
19-26: LGTM on temp file handling.Good use of per-user temp directory (
gradata-{uid}) with propermkdir(parents=True, exist_ok=True). The atomic write pattern with temp file + rename is the correct approach for concurrent safety.tests/test_self_healing.py (2)
1-493: Comprehensive test coverage for self-healing engine!Excellent test suite covering:
- Rule failure detection with various edge cases (low confidence, killed rules, multi-match)
patch_rule()behavior including metadata preservation and event emission- Retroactive test validation including delta-based matching
- Nudge threshold with auto-created INSTINCT lessons
- Scope narrowing with domain exclusions
- Full E2E integration test
The use of
tmp_pathfixture for brain initialization avoids hardcoded paths.
192-194: No issue found.Line 192's use of
l.state.value == "RULE"is correct. TheLessonStateenum definesRULE = "RULE"(a string value, not numeric), so.valuereturns"RULE"as expected. While.nameis also used elsewhere in the codebase for the same purpose, both approaches work identically for this enum structure and there is no inconsistency or risk of silent failure.> Likely an incorrect or invalid review comment.src/gradata/hooks/_base.py (4)
1-22: LGTM! Module structure and imports are correct.The module correctly includes
from __future__ import annotationsand establishes proper logging. Theprint()usage in this hooks module is appropriate since hooks communicate with Claude Code via stdout JSON protocol.
24-31: LGTM! Profile gating logic is correct.The hierarchical comparison
get_profile() >= min_profilecorrectly ensures that higher-tier profiles (STRICT) include all hooks from lower tiers (STANDARD, MINIMAL).
34-40: LGTM!Silent JSON parsing failure is appropriate for hooks that must not block the tool chain.
51-81: LGTM! Brain resolution with proper fallback chain.The resolution order (env vars → default location) and silent failure modes are appropriate for hook execution.
src/gradata/hooks/_installer.py (7)
22-40: LGTM! Well-structured hook registry.The registry correctly maps hooks to their events, matchers, profile tiers, and timeouts. The consistent "Gradata:" prefix in descriptions enables reliable hook identification.
49-78: LGTM! Settings generation logic is correct.The profile filtering (
min_profile > max_profile) correctly excludes hooks above the requested tier, andsys.executableensures the correct Python interpreter is used.
85-99: LGTM! Robust settings I/O with proper error handling.The fallback to empty dict on parse errors and parent directory creation are appropriate defensive measures.
102-111: LGTM! Comprehensive hook identification.The detection covers both description patterns and command strings, ensuring reliable identification of Gradata hooks.
118-138: LGTM! Clean install workflow.The pattern of removing existing Gradata hooks before installing new ones prevents duplication while preserving user's non-Gradata hooks.
152-164: LGTM! Safe dictionary modification during iteration.Using
list(hooks.keys())correctly prevents modification during iteration, and empty event cleanup keeps settings tidy.
167-192: LGTM! Informative status output.The status function provides clear feedback about installed hooks and guidance when none are found.
src/gradata/enhancements/self_healing.py (8)
1-23: LGTM! Module setup and constants are correct.The module includes required
from __future__ import annotations, andDEFAULT_MIN_CONFIDENCE = 0.80is within the valid [0.0, 1.0] range per coding guidelines.
25-76: LGTM! Rule failure detection logic is correct.The function correctly identifies when a high-confidence RULE should have prevented a correction. The guard on line 60 ensures
max()won't fail on an empty list.
99-157: LGTM! Retroactive test with dual scoring approach.The combination of TF-IDF similarity and stem overlap provides good coverage for both long and short texts. The 0.20 threshold is appropriately low since this is just the first gate before graduation.
160-185: LGTM! Deterministic patch generation heuristic.The simple word-difference approach with stop word filtering produces reasonable qualifying clauses. Sorting ensures deterministic output for testing.
188-224: LGTM! Batch processing of rule failures.The function cleanly processes failure events into testable patch candidates, allowing downstream filtering on
retroactive_testresults.
232-246: LGTM! Centroid finding with proper edge case handling.The O(n²) pairwise comparison is acceptable given the low correction threshold. Edge cases for empty and single-element lists are handled correctly.
336-389: Clarification: Function returns scope but doesn't apply it.The function computes the new
scope_jsonbut doesn't mutaterule.scope_json. This appears intentional (Phase 2 prep per docstring), but ensure callers understand they must apply the returnednew_scope_jsonthemselves.
249-331: INITIAL_CONFIDENCE is compliant with confidence value requirements.The function correctly uses
INITIAL_CONFIDENCE = 0.40from theself_improvementmodule, which falls within the required [0.0, 1.0] range per coding guidelines.src/gradata/enhancements/meta_rules_storage.py (2)
379-379: Good schema bootstrap before single-row upsert.Adding
ensure_pattern_table(db_path)on Line 379 prevents first-write failures on fresh databases and makes this API safer to call directly.
411-411: Batch path now correctly bootstraps DDL.The Line 411 guard aligns batch behavior with single upsert and avoids
no such tablefailures in clean environments.
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v4 | ||
| with: | ||
| version: "latest" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
What is the latest stable version of uv (astral-sh)?
💡 Result:
The latest stable release of uv (astral-sh) is 0.11.6, released April 9, 2026. [1]
Sources:
[1] (pypi.org)
Citations:
Pin the uv version for reproducible builds.
Using version: "latest" can cause non-deterministic CI behavior if uv releases breaking changes. Pin to a specific version (e.g., "0.11.6") or a version range (e.g., "0.11.x") to ensure consistent builds across runs.
♻️ Suggested fix
- name: Install uv
uses: astral-sh/setup-uv@v4
with:
- version: "latest"
+ version: "0.11.x"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install uv | |
| uses: astral-sh/setup-uv@v4 | |
| with: | |
| version: "latest" | |
| - name: Install uv | |
| uses: astral-sh/setup-uv@v4 | |
| with: | |
| version: "0.11.x" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/sdk-ci.yml around lines 32 - 35, Replace the
non-deterministic version pin in the GitHub Actions step that installs uv (the
step named "Install uv" which uses astral-sh/setup-uv@v4) so it no longer uses
version: "latest"; update the with.version value to a specific release (e.g.,
"0.11.6") or a stable semver range (e.g., "0.11.x") to ensure reproducible CI
runs.
| nav: | ||
| - Home: index.md | ||
| - Getting Started: | ||
| - Installation: getting-started/install.md | ||
| - Quick Start: getting-started/quickstart.md | ||
| - Core Concepts: getting-started/concepts.md | ||
| - Gradata Cloud: | ||
| - Overview: cloud/overview.md | ||
| - Getting Started: cloud/quickstart.md | ||
| - API Reference: cloud/api.md | ||
| - Marketplace: cloud/marketplace.md | ||
| - Guides: | ||
| - Training a Brain: guides/training.md | ||
| - Custom Domains: guides/custom-domains.md | ||
| - MCP Integration: guides/mcp.md | ||
| - API Reference: | ||
| - Brain Class: api/brain.md | ||
| - CLI: api/cli.md | ||
| - Contributing: | ||
| - Architecture: architecture/overview.md | ||
| - Patterns: architecture/patterns.md | ||
| - Enhancements: architecture/enhancements.md | ||
| - Brain Data: architecture/brain.md | ||
| - Agent Adaptation: architecture/agent-graduation.md | ||
| - Patterns API: api/patterns.md | ||
| - Enhancements API: api/enhancements.md | ||
| - Agent Adaptation Guide: guides/agent-graduation.md |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if all markdown files referenced in the navigation exist
cd sdk
# List of all files referenced in the navigation
files=(
"docs/index.md"
"docs/getting-started/install.md"
"docs/getting-started/quickstart.md"
"docs/getting-started/concepts.md"
"docs/cloud/overview.md"
"docs/cloud/quickstart.md"
"docs/cloud/api.md"
"docs/cloud/marketplace.md"
"docs/guides/training.md"
"docs/guides/custom-domains.md"
"docs/guides/mcp.md"
"docs/guides/agent-graduation.md"
"docs/api/brain.md"
"docs/api/cli.md"
"docs/api/patterns.md"
"docs/api/enhancements.md"
"docs/architecture/overview.md"
"docs/architecture/patterns.md"
"docs/architecture/enhancements.md"
"docs/architecture/brain.md"
"docs/architecture/agent-graduation.md"
)
missing=()
for file in "${files[@]}"; do
if [[ ! -f "$file" ]]; then
missing+=("$file")
fi
done
if [[ ${`#missing`[@]} -eq 0 ]]; then
echo "✅ All navigation files exist"
else
echo "❌ Missing ${`#missing`[@]} file(s):"
printf ' - %s\n' "${missing[@]}"
exit 1
fiRepository: Gradata/gradata
Length of output: 714
🏁 Script executed:
#!/bin/bash
# List the actual structure of the sdk directory
echo "=== sdk directory structure ==="
ls -la sdk/
echo -e "\n=== Check if docs directory exists in sdk ==="
if [ -d "sdk/docs" ]; then
echo "docs/ directory EXISTS"
echo "Contents of sdk/docs/:"
find sdk/docs -type f -name "*.md" 2>/dev/null | head -30
else
echo "docs/ directory DOES NOT exist"
fi
echo -e "\n=== Search for all .md files in sdk ==="
find sdk -type f -name "*.md" 2>/dev/null | sortRepository: Gradata/gradata
Length of output: 383
Create missing documentation files referenced in the navigation.
All 21 markdown files referenced in the navigation (lines 30-56) do not exist. The docs/ directory is missing entirely, and the documentation build will fail without these files. Create the required directory structure and documentation files:
docs/index.mddocs/getting-started/{install,quickstart,concepts}.mddocs/cloud/{overview,quickstart,api,marketplace}.mddocs/guides/{training,custom-domains,mcp,agent-graduation}.mddocs/api/{brain,cli,patterns}.mddocs/architecture/{overview,patterns,enhancements,brain,agent-graduation}.md
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/mkdocs.yml` around lines 30 - 56, The mkdocs navigation references many
missing documentation pages so the site build will fail; create the docs
directory and add the exact markdown files referenced by the nav entries (e.g.,
index.md; getting-started/install.md, getting-started/quickstart.md,
getting-started/concepts.md; cloud/overview.md, cloud/quickstart.md,
cloud/api.md, cloud/marketplace.md; guides/training.md,
guides/custom-domains.md, guides/mcp.md, guides/agent-graduation.md;
api/brain.md, api/cli.md, api/patterns.md; architecture/overview.md,
architecture/patterns.md, architecture/enhancements.md, architecture/brain.md,
architecture/agent-graduation.md) with at least a frontmatter/title and
placeholder content so mkdocs can build, ensuring file names and directory
structure match the nav keys (e.g., "Getting Started" => getting-started/*,
"Gradata Cloud" => cloud/*, etc.).
| plugins: | ||
| - search | ||
| - mkdocstrings: | ||
| handlers: | ||
| python: | ||
| paths: [src] | ||
| options: | ||
| show_source: false | ||
| show_root_heading: true | ||
| heading_level: 2 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that Python source files exist at the configured path
if [[ -d "sdk/src" ]]; then
echo "✅ sdk/src directory exists"
echo ""
echo "Python files found:"
fd -e py . sdk/src -x echo " - {}"
# Check if there are any Python files
if [[ $(fd -e py . sdk/src | wc -l) -eq 0 ]]; then
echo ""
echo "⚠️ No Python files found in sdk/src"
exit 1
fi
else
echo "❌ sdk/src directory does not exist"
exit 1
fiRepository: Gradata/gradata
Length of output: 93
🏁 Script executed:
# Find Python files and directory structure in the sdk directory
find sdk -type f -name "*.py" | head -20Repository: Gradata/gradata
Length of output: 41
🏁 Script executed:
# Check the overall structure of the sdk directory
ls -la sdk/Repository: Gradata/gradata
Length of output: 232
🏁 Script executed:
# Search for common Python package locations
fd -t d "^(src|python|lib|packages)$" sdk/Repository: Gradata/gradata
Length of output: 41
🏁 Script executed:
# Search for Python files across the entire repository
find . -type f -name "*.py" -not -path "*/\.*" | head -20Repository: Gradata/gradata
Length of output: 636
🏁 Script executed:
# Check if there's a src directory at repository root
ls -la | grep -E "^d.*src$|^d.*python"Repository: Gradata/gradata
Length of output: 112
🏁 Script executed:
# Check the overall repository structure at root
ls -laRepository: Gradata/gradata
Length of output: 1270
🏁 Script executed:
# Check the root mkdocs.yml to understand the documentation structure
cat mkdocs.yml | head -30Repository: Gradata/gradata
Length of output: 758
🏁 Script executed:
# Check if there's a build script or CI configuration that runs mkdocs
find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "mkdocs" 2>/dev/nullRepository: Gradata/gradata
Length of output: 117
🏁 Script executed:
# Check the GitHub workflow to see how mkdocs is executed
cat .github/workflows/docs.ymlRepository: Gradata/gradata
Length of output: 1089
🏁 Script executed:
# Search for any references to sdk/mkdocs.yml in the codebase
rg "sdk.*mkdocs|mkdocs.*sdk" --type yaml --type md --type txtRepository: Gradata/gradata
Length of output: 150
🏁 Script executed:
# Check if there's any build documentation or README about sdk
grep -r "sdk" README.md CONTRIBUTING.md 2>/dev/null | head -20Repository: Gradata/gradata
Length of output: 41
🏁 Script executed:
# Try a simpler search for sdk references
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*.yml" \) ! -path "*/.git/*" -exec grep -l "sdk" {} \;Repository: Gradata/gradata
Length of output: 119
🏁 Script executed:
# Check if there are any files inside the sdk directory beyond mkdocs.yml
find sdk -type fRepository: Gradata/gradata
Length of output: 73
🏁 Script executed:
# Check git status of sdk directory to understand its state
git ls-files sdk/Repository: Gradata/gradata
Length of output: 73
🏁 Script executed:
# Check the SDK CI workflow to see if it uses mkdocs
cat .github/workflows/sdk-ci.ymlRepository: Gradata/gradata
Length of output: 1858
🏁 Script executed:
# Check if there's a pyproject.toml in the sdk directory
cat sdk/pyproject.toml 2>/dev/null || echo "No pyproject.toml in sdk/"Repository: Gradata/gradata
Length of output: 84
🏁 Script executed:
# List all files in the sdk directory recursively
find sdk -type fRepository: Gradata/gradata
Length of output: 73
Fix the SDK source path configuration — the mkdocstrings path is incorrect, and the broader SDK CI will fail.
The paths: [src] in mkdocstrings is configured to look for sdk/src, but this directory does not exist. The actual Python source is located at ../src relative to the sdk directory (or ./src from the repository root). Additionally, the SDK CI workflow (.github/workflows/sdk-ci.yml) also references src/gradata/ from the sdk directory, which means both the documentation and CI tasks will fail.
Update paths: [../src] in sdk/mkdocs.yml to correctly reference the Python source location, or reconfigure the SDK directory structure if it should be independent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/mkdocs.yml` around lines 58 - 67, The mkdocstrings handler in mkdocs
config currently points to the wrong source (paths: [src]) causing docs/CI to
look for sdk/src; update the mkdocstrings paths setting (under the mkdocstrings
-> handlers -> python -> paths key) to paths: [../src] so it correctly
references the repository Python package (or alternatively adjust SDK layout),
and ensure this matches how the SDK CI references src/gradata so both docs
rendering and CI use the same ../src location.
| for finding in findings: | ||
| for f in finding.get("files", []): | ||
| if Path(f).name == file_basename: | ||
| # User is editing a file related to a test finding | ||
| _save_findings([]) # Clear acted-on findings | ||
| return {"result": "Correction captured from test finding"} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clearing all findings on any single match may be too aggressive.
When a user edits a file matching any finding, all findings are cleared. If multiple test failures exist across different files, fixing one file clears tracking for others that haven't been addressed yet.
Consider clearing only the matched finding(s) instead:
Proposed fix to clear only matched findings
file_basename = Path(file_path).name
+ matched_indices = []
- for finding in findings:
- for f in finding.get("files", []):
+ for idx, finding in enumerate(findings):
+ for f in finding.get("files", []):
if Path(f).name == file_basename:
- # User is editing a file related to a test finding
- _save_findings([]) # Clear acted-on findings
- return {"result": "Correction captured from test finding"}
+ matched_indices.append(idx)
+ break
+
+ if matched_indices:
+ # Remove only the matched findings, preserve others
+ remaining = [f for i, f in enumerate(findings) if i not in matched_indices]
+ _save_findings(remaining)
+ return {"result": "Correction captured from test finding"}
return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/hooks/tool_finding_capture.py` around lines 116 - 121, The
current loop in tool_finding_capture.py clears all findings by calling
_save_findings([]) as soon as any file basename matches; change this to remove
only the matched finding(s) from the findings list and persist the reduced list
via _save_findings(updated_findings). Specifically, inside the nested loops
where you iterate over findings and each finding.get("files", []), detect
matches using Path(f).name == file_basename, collect the matching finding ids or
indices (or filter by identity), build a new list excluding those matched
entries, then call _save_findings(new_findings) and return the same result
string; keep the existing variables (findings, file_basename, Path,
_save_findings) to locate and implement the change.
| def test_get_profile_default(): | ||
| with patch.dict(os.environ, {}, clear=True): | ||
| os.environ.pop("GRADATA_HOOK_PROFILE", None) | ||
| assert get_profile() == Profile.STANDARD |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant os.environ.pop() after clear=True.
When using patch.dict(os.environ, {}, clear=True), the environment is already cleared. The subsequent pop() is unnecessary and could be confusing.
Simplified version
def test_get_profile_default():
- with patch.dict(os.environ, {}, clear=True):
- os.environ.pop("GRADATA_HOOK_PROFILE", None)
+ with patch.dict(os.environ, {"GRADATA_HOOK_PROFILE": ""}, clear=True):
+ os.environ.pop("GRADATA_HOOK_PROFILE", None) # Ensure key is absent
assert get_profile() == Profile.STANDARDOr simply rely on clear=True:
def test_get_profile_default():
- with patch.dict(os.environ, {}, clear=True):
- os.environ.pop("GRADATA_HOOK_PROFILE", None)
+ with patch.dict(os.environ, {}, clear=True):
assert get_profile() == Profile.STANDARD🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_hooks_base.py` around lines 24 - 27, The test
test_get_profile_default contains a redundant
os.environ.pop("GRADATA_HOOK_PROFILE", None) after using patch.dict(os.environ,
{}, clear=True); remove the pop call so the test simply clears the environment
with patch.dict and asserts get_profile() == Profile.STANDARD, keeping
references to test_get_profile_default, patch.dict, get_profile, and
Profile.STANDARD to locate the change.
| def test_agent_precontext_no_brain(): | ||
| with patch.dict(os.environ, {"GRADATA_BRAIN_DIR": "/nonexistent/path/xyz"}): | ||
| result = precontext_main({ | ||
| "tool_name": "Agent", | ||
| "tool_input": {"subagent_type": "general"}, | ||
| }) | ||
| assert result is None |
There was a problem hiding this comment.
Replace hardcoded nonexistent paths with tmp_path-based ones.
These tests hardcode POSIX-only missing paths to simulate “no brain”, which makes the suite less portable and violates the test-path rule. Use a non-created tmp_path / "missing" path instead.
As per coding guidelines, tests/**: Test files. Verify: no hardcoded paths.
Also applies to: 76-83, 262-265, 346-349, 377-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_hooks_intelligence.py` around lines 31 - 37, The test uses a
hardcoded POSIX nonexistent path when calling precontext_main in
test_agent_precontext_no_brain; change it to use the pytest tmp_path fixture
(e.g., str(tmp_path / "missing")) and patch.dict with that string so the path is
platform-independent; apply the same replacement for the other tests referenced
(lines around 76-83, 262-265, 346-349, 377-380) that currently hardcode
"/nonexistent/..." paths and ensure test functions accept the tmp_path fixture
where needed.
| assert rules_and_patterns[0].state.name == "RULE" | ||
| assert rules_and_patterns[0].confidence == 0.92 | ||
| assert rules_and_patterns[1].state.name == "PATTERN" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use pytest.approx for floating-point comparisons.
Direct equality comparison of floats can be fragile due to precision issues, even though in this case the value is parsed from a string.
♻️ Proposed fix
+import pytest
+
def test_parse_lessons_extracts_rules():
...
- assert rules_and_patterns[0].confidence == 0.92
+ assert rules_and_patterns[0].confidence == pytest.approx(0.92)As per coding guidelines: "floating point comparisons use pytest.approx".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_hooks_learning.py` around lines 22 - 24, The test does a direct
float equality check on rules_and_patterns[0].confidence which is fragile;
change the assertion to use pytest.approx for robust comparison (e.g., assert
rules_and_patterns[0].confidence == pytest.approx(0.92)) and ensure pytest is
available in the test namespace (import pytest if not already present) so that
the confidence comparison in tests/test_hooks_learning.py uses pytest.approx
rather than ==.
| def test_secret_scan_blocks_openai_key(): | ||
| data = { | ||
| "tool_name": "Write", | ||
| "tool_input": {"file_path": "config.py", "content": "key = 'sk-abc123def456ghi789jkl012mno345pqr'"}, | ||
| } | ||
| result = scan_main(data) | ||
| assert result is not None | ||
| assert result["decision"] == "block" | ||
| assert "SECRET" in result["reason"] | ||
|
|
||
|
|
||
| def test_secret_scan_blocks_aws_key(): | ||
| data = { | ||
| "tool_name": "Edit", | ||
| "tool_input": {"new_string": "AKIAIOSFODNN7EXAMPLE"}, | ||
| } | ||
| result = scan_main(data) | ||
| assert result is not None | ||
| assert result["decision"] == "block" | ||
|
|
||
|
|
||
| def test_secret_scan_allows_clean_code(): | ||
| data = { | ||
| "tool_name": "Write", | ||
| "tool_input": {"file_path": "main.py", "content": "print('hello world')"}, | ||
| } | ||
| result = scan_main(data) | ||
| assert result is None | ||
|
|
||
|
|
||
| def test_secret_scan_no_content(): | ||
| data = {"tool_name": "Write", "tool_input": {"file_path": "empty.py"}} | ||
| result = scan_main(data) | ||
| assert result is None | ||
|
|
||
|
|
||
| def test_secret_scan_blocks_private_key(): | ||
| data = { | ||
| "tool_name": "Write", | ||
| "tool_input": {"file_path": "key.pem", "content": "-----BEGIN RSA PRIVATE KEY-----\nfoo\n-----END RSA PRIVATE KEY-----"}, | ||
| } | ||
| result = scan_main(data) | ||
| assert result is not None | ||
| assert result["decision"] == "block" | ||
|
|
||
|
|
||
| def test_secret_scan_blocks_db_connection(): | ||
| data = { | ||
| "tool_name": "Write", | ||
| "tool_input": {"file_path": "db.py", "content": "postgres://user:s3cret@db.host.com/mydb"}, | ||
| } | ||
| result = scan_main(data) | ||
| assert result is not None | ||
| assert result["decision"] == "block" | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Static analysis false positive — this is intentional test data.
The Betterleaks warning on line 16 is a false positive. The "API key" (sk-abc123def456ghi789jkl012mno345pqr) is a dummy test value specifically used to verify the secret scanner blocks such patterns. The key is not a real credential.
Optional: Consider parametrizing similar test cases for secret scanning:
♻️ Optional parametrization
import pytest
`@pytest.mark.parametrize`("content,should_block", [
("key = 'sk-abc123def456ghi789jkl012mno345pqr'", True),
("AKIAIOSFODNN7EXAMPLE", True),
("-----BEGIN RSA PRIVATE KEY-----\nfoo\n-----END RSA PRIVATE KEY-----", True),
("postgres://user:s3cret@db.host.com/mydb", True),
("print('hello world')", False),
])
def test_secret_scan_detection(content, should_block):
data = {"tool_name": "Write", "tool_input": {"file_path": "test.py", "content": content}}
result = scan_main(data)
if should_block:
assert result is not None and result["decision"] == "block"
else:
assert result is None🧰 Tools
🪛 Betterleaks (1.1.1)
[high] 16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_hooks_safety.py` around lines 13 - 67, The test contains an
intentional dummy secret in test_secret_scan_blocks_openai_key that triggers a
static-analysis false positive; to fix, mark that test value as test-only so the
scanner ignores it by adding an inline suppression/comment on the content string
(e.g., append a recognized ignore token like "# nosec" or a project-specific
Betterleaks suppression comment and a short note "dummy test key"), or
alternatively consolidate cases by parametrizing the tests into a single
test_secret_scan_detection that iterates (content, should_block) and calls
scan_main, as shown in the reviewer example; update the test names
(test_secret_scan_blocks_openai_key or test_secret_scan_detection) and the
scan_main usage accordingly so the intent and suppression are clear.
Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gradata/enhancements/behavioral_extractor.py`:
- Around line 34-39: The current try/except around "from
gradata.enhancements.edit_classifier import _FACTUAL_RE" swallows ImportError
even when the module fails to import for other reasons; change the import
handling so that you only fall back to the regex when the module truly does not
exist. Use importlib.util.find_spec("gradata.enhancements.edit_classifier") (or
similar) to detect absence: if the spec is None, set the fallback _FACTUAL_RE;
otherwise re-raise the original import exception so real import errors in
gradata.enhancements.edit_classifier are not masked. Ensure the code still
defines _FACTUAL_RE when the module is missing but does not hide other failures.
- Around line 427-433: The _is_actionable check is too permissive: replace the
broad "capitalized and 3+ words" rule in _is_actionable with a stricter
first-word verb check — e.g., create a small whitelist like _IMPERATIVE_VERBS
(or use the project POS-tagger if available) and change the second branch to
return True only when first_word.lower() is in _IMPERATIVE_VERBS (or when POS
tagger marks the first token as a verb), while keeping the existing
_IMPERATIVE_STARTERS/PREFIX_INSTRUCTION acceptance; update the logic in
_is_actionable accordingly to avoid accepting generic capitalized declaratives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 633359eb-4566-4d14-a19d-a0a8309038f3
📒 Files selected for processing (3)
src/gradata/enhancements/behavioral_extractor.pysrc/gradata/enhancements/meta_rules_storage.pytests/test_pipeline_e2e.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (2)
tests/**
⚙️ CodeRabbit configuration file
tests/**: Test files. Verify: no hardcoded paths, assertions check specific values not just truthiness,
parametrized tests preferred for boundary conditions, floating point comparisons use pytest.approx.
Files:
tests/test_pipeline_e2e.py
src/gradata/**/*.py
⚙️ CodeRabbit configuration file
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required), no print()
statements (use logging), all functions accepting BrainContext where DB access occurs, no hardcoded paths. Severity
scoring must clamp to [0,1]. Confidence values must be in [0.0, 1.0].
Files:
src/gradata/enhancements/meta_rules_storage.pysrc/gradata/enhancements/behavioral_extractor.py
🔇 Additional comments (6)
src/gradata/enhancements/meta_rules_storage.py (3)
379-379: Good fix: bootstrap table before single-pattern upsert.Adding
ensure_pattern_table(db_path)at Line 379 prevents fresh-database write failures and hardens this path.
411-411: Good fix: bootstrap table before batch upsert.Line 411 correctly makes the batch path resilient on uninitialized DBs as well.
491-491: Connection cleanup is correctly preserved.Keeping close logic in
finallyat Line 491 ensures deterministic cleanup on both success and failure.tests/test_pipeline_e2e.py (1)
305-305: Assertion is aligned with gate semantics.Line 305 validates the minimum weighted-score threshold expected by
min_score, which is appropriate for this e2e check.src/gradata/enhancements/behavioral_extractor.py (2)
209-223: Good improvements to deterministic matching and normalization.Line 209/210 avoids repeated lowercasing, Line 212/223 uses bounded regex checks, Line 406/407 expands imperative starters, and Line 514 normalizes category fallback lookup cleanly.
Also applies to: 406-407, 442-447, 514-514
205-206: No action required. The project declaresrequires-python = ">=3.11"in pyproject.toml, which exceeds the Python 3.10+ requirement forzip(..., strict=True). All three usages (lines 205, 252, 290) are compatible with the declared minimum Python version.
| # Import at module level to avoid private-symbol import inside function body | ||
| try: | ||
| from gradata.enhancements.edit_classifier import _FACTUAL_RE | ||
| except ImportError: | ||
| _FACTUAL_RE = re.compile(r"\b\d[\d,.]*\b|\b\d{1,2}[/-]\d{1,2}[/-]\d{2,4}\b|https?://\S+") | ||
|
|
There was a problem hiding this comment.
Avoid masking real import failures in _FACTUAL_RE fallback.
Line 35 catches ImportError broadly; if gradata.enhancements.edit_classifier fails to import for unrelated reasons, behavior silently degrades to the fallback regex.
Proposed fix
-try:
- from gradata.enhancements.edit_classifier import _FACTUAL_RE
-except ImportError:
- _FACTUAL_RE = re.compile(r"\b\d[\d,.]*\b|\b\d{1,2}[/-]\d{1,2}[/-]\d{2,4}\b|https?://\S+")
+from gradata.enhancements import edit_classifier as _edit_classifier
+
+if hasattr(_edit_classifier, "_FACTUAL_RE"):
+ _FACTUAL_RE = _edit_classifier._FACTUAL_RE
+else:
+ _FACTUAL_RE = re.compile(r"\b\d[\d,.]*\b|\b\d{1,2}[/-]\d{1,2}[/-]\d{2,4}\b|https?://\S+")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Import at module level to avoid private-symbol import inside function body | |
| try: | |
| from gradata.enhancements.edit_classifier import _FACTUAL_RE | |
| except ImportError: | |
| _FACTUAL_RE = re.compile(r"\b\d[\d,.]*\b|\b\d{1,2}[/-]\d{1,2}[/-]\d{2,4}\b|https?://\S+") | |
| # Import at module level to avoid private-symbol import inside function body | |
| from gradata.enhancements import edit_classifier as _edit_classifier | |
| if hasattr(_edit_classifier, "_FACTUAL_RE"): | |
| _FACTUAL_RE = _edit_classifier._FACTUAL_RE | |
| else: | |
| _FACTUAL_RE = re.compile(r"\b\d[\d,.]*\b|\b\d{1,2}[/-]\d{1,2}[/-]\d{2,4}\b|https?://\S+") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/enhancements/behavioral_extractor.py` around lines 34 - 39, The
current try/except around "from gradata.enhancements.edit_classifier import
_FACTUAL_RE" swallows ImportError even when the module fails to import for other
reasons; change the import handling so that you only fall back to the regex when
the module truly does not exist. Use
importlib.util.find_spec("gradata.enhancements.edit_classifier") (or similar) to
detect absence: if the spec is None, set the fallback _FACTUAL_RE; otherwise
re-raise the original import exception so real import errors in
gradata.enhancements.edit_classifier are not masked. Ensure the code still
defines _FACTUAL_RE when the module is missing but does not hide other failures.
| if first_word in _IMPERATIVE_STARTERS: | ||
| return True | ||
| # Accept instructions from PREFIX_INSTRUCTION archetype (explicit user rules) | ||
| # and any instruction that looks imperative (capitalized verb form) | ||
| if instruction[0].isupper() and len(instruction.split()) >= 3: | ||
| return True | ||
| return False |
There was a problem hiding this comment.
_is_actionable became too permissive and can admit non-imperative text.
Line 431 accepts any capitalized 3+ word sentence, so declarative strings can pass the gate and pollute extracted rules.
Proposed fix
- # Accept instructions from PREFIX_INSTRUCTION archetype (explicit user rules)
- # and any instruction that looks imperative (capitalized verb form)
- if instruction[0].isupper() and len(instruction.split()) >= 3:
- return True
+ lowered = instruction.lower()
+ if lowered.startswith(("please ", "remember to ", "make sure to ")):
+ return True
return False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/enhancements/behavioral_extractor.py` around lines 427 - 433, The
_is_actionable check is too permissive: replace the broad "capitalized and 3+
words" rule in _is_actionable with a stricter first-word verb check — e.g.,
create a small whitelist like _IMPERATIVE_VERBS (or use the project POS-tagger
if available) and change the second branch to return True only when
first_word.lower() is in _IMPERATIVE_VERBS (or when POS tagger marks the first
token as a verb), while keeping the existing
_IMPERATIVE_STARTERS/PREFIX_INSTRUCTION acceptance; update the logic in
_is_actionable accordingly to avoid accepting generic capitalized declaratives.
| def _findings_path() -> Path: | ||
| uid = os.getuid() if hasattr(os, "getuid") else "win" | ||
| user_tmp = Path(tempfile.gettempdir()) / f"gradata-{uid}" | ||
| user_tmp.mkdir(parents=True, exist_ok=True) | ||
| return user_tmp / "findings.json" | ||
|
|
||
|
|
||
| FINDINGS_FILE = _findings_path() |
There was a problem hiding this comment.
Module-level
mkdir side effect causes import failures in restricted environments
FINDINGS_FILE = _findings_path() is evaluated at import time. _findings_path() unconditionally calls user_tmp.mkdir(parents=True, exist_ok=True), so every import tool_finding_capture creates a directory on disk as a side effect. In CI runners, Docker containers, or read-only sandboxes where tempfile.gettempdir() is not writable, this raises an OSError and turns the import into a module-level exception — crashing the hook process entirely.
Additionally, locking the path as a module-level constant means tests that mock tempfile.gettempdir() after import won't affect the already-resolved path.
Defer the path resolution to call time:
def _findings_path() -> Path:
uid = os.getuid() if hasattr(os, "getuid") else "win"
user_tmp = Path(tempfile.gettempdir()) / f"gradata-{uid}"
user_tmp.mkdir(parents=True, exist_ok=True)
return user_tmp / "findings.json"
# Remove the module-level call; use _findings_path() directly at call sitesThen replace all uses of FINDINGS_FILE with _findings_path(), or use functools.lru_cache on _findings_path to cache after first successful resolution.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gradata/hooks/tool_finding_capture.py
Line: 19-26
Comment:
**Module-level `mkdir` side effect causes import failures in restricted environments**
`FINDINGS_FILE = _findings_path()` is evaluated at import time. `_findings_path()` unconditionally calls `user_tmp.mkdir(parents=True, exist_ok=True)`, so every `import tool_finding_capture` creates a directory on disk as a side effect. In CI runners, Docker containers, or read-only sandboxes where `tempfile.gettempdir()` is not writable, this raises an `OSError` and turns the import into a module-level exception — crashing the hook process entirely.
Additionally, locking the path as a module-level constant means tests that mock `tempfile.gettempdir()` after import won't affect the already-resolved path.
Defer the path resolution to call time:
```python
def _findings_path() -> Path:
uid = os.getuid() if hasattr(os, "getuid") else "win"
user_tmp = Path(tempfile.gettempdir()) / f"gradata-{uid}"
user_tmp.mkdir(parents=True, exist_ok=True)
return user_tmp / "findings.json"
# Remove the module-level call; use _findings_path() directly at call sites
```
Then replace all uses of `FINDINGS_FILE` with `_findings_path()`, or use `functools.lru_cache` on `_findings_path` to cache after first successful resolution.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (16)
src/gradata/_doctor.py (1)
246-264:⚠️ Potential issue | 🟠 MajorReplace
print()-based output with logger calls in core SDK module.
print_diagnosisstill emits output viaprint()(including Line 264). This violates the core SDK logging rule and should be migrated to structured logging (or return formatted text and let CLI print).As per coding guidelines, "
src/gradata/**/*.py: ... no print() statements (use logging)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/_doctor.py` around lines 246 - 264, The print_diagnosis function in src/gradata/_doctor.py currently calls print() for all output; replace these prints with structured logging (e.g., get a module logger via logging.getLogger(__name__) and emit logger.info for normal lines, logger.warning for "degraded" messages and logger.error for the failing status) or alternatively return the formatted multi-line string from print_diagnosis and let the CLI layer print it; update the loop over report["checks"] to log each check with icons.get(check["status"]), the humanized name and detail instead of printing, and ensure the final status block uses the appropriate logger level (healthy → info, degraded → warning, else → error) so no print() calls remain in the module.src/gradata/__init__.py (1)
1-26:⚠️ Potential issue | 🟠 MajorAdd postponed-annotations import to satisfy core SDK typing rule.
This module is missing
from __future__ import annotations, which is required for files undersrc/gradata/**/*.py.✅ Proposed fix
""" Gradata — Procedural memory for AI agents. @@ """ +from __future__ import annotations __version__ = "0.2.0"As per coding guidelines, "
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/__init__.py` around lines 1 - 26, Add the postponed-annotations future import at the very top of the module: insert from __future__ import annotations as the first statement in this file (before the module docstring and any other imports) so the typing rule is satisfied for the core SDK; keep the rest of the file (the module docstring, __version__, and the logging/os imports) unchanged.src/gradata/contrib/enhancements/install_manifest.py (1)
499-499: 🧹 Nitpick | 🔵 TrivialConsider moving the
timeimport to module level.The function-level import works but is unconventional. Moving standard library imports to the top of the file is more idiomatic Python.
♻️ Move import to module level
At the top of the file, add
timeto the imports (around line 30):from __future__ import annotations import json +import time from dataclasses import asdict, dataclass, fieldThen remove the function-level import:
) -> InstallState: """Apply an install plan and return updated state. This is a logical operation — actual module loading is handled by the caller. This method tracks what's installed. Args: plan: The install plan to apply. current_state: Current install state (for diff-based updates). Returns: Updated InstallState. """ - import time - state = current_state or InstallState()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/contrib/enhancements/install_manifest.py` at line 499, Move the function-local "import time" into the module-level imports in install_manifest.py: add time to the existing top-of-file imports alongside the other standard-library imports and remove the in-function "import time" statement so the module uses the top-level time import throughout (locate the in-function import in the function where "import time" currently appears and delete it after adding the module-level import).src/gradata/enhancements/git_backfill.py (1)
190-190: 🧹 Nitpick | 🔵 TrivialConsider strengthening type safety for the
brainparameter.The
brainparameter is typed asAny, which weakens type guarantees. Consider using a more specific type annotation, such as a Protocol defining the requiredcorrect()method or a forward reference to the Brain class if available.Example with Protocol:
from typing import Protocol class BrainLike(Protocol): def correct(self, draft: str, final: str, session: int | None = None) -> dict[str, Any]: ...Then update the signature:
def backfill_from_git( - brain: Any, + brain: BrainLike, repo_path: str | Path = ".",As per coding guidelines: "Check for: type safety" for files in
src/gradata/**/*.py.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/git_backfill.py` at line 190, The parameter brain currently typed as Any should use a stricter type: define a Protocol (e.g., BrainLike) that declares the methods used (at least correct(self, draft: str, final: str, session: int | None = None) -> dict[str, Any]) or, if a concrete Brain class exists, use a forward reference to that class; then update the function/method signature that contains "brain: Any" in git_backfill.py to use BrainLike or "Brain" instead and import typing.Protocol and typing.Any as needed so type checkers can validate usages of brain.correct and related calls.src/gradata/enhancements/quality_monitoring.py (1)
177-178:⚠️ Potential issue | 🟡 MinorFix the message format to correctly show the direction of change.
The message describes a drop in corrections but displays the values backwards. When corrections drop from
previous(higher) tocurrent(lower), the arrow should showprevious → current, notcurrent → previous.For example, if
previous.correction_density=0.80andcurrent.correction_density=0.30, the current message would read "Corrections dropped 0.30 → 0.80", which looks like an increase, not a drop.🔄 Proposed fix
- message=f"Corrections dropped {current.correction_density:.2f} → {previous.correction_density:.2f} but edit distance unchanged ({current.edit_distance_avg:.2f}). Brain may be ignored.")] + message=f"Corrections dropped {previous.correction_density:.2f} → {current.correction_density:.2f} but edit distance unchanged ({current.edit_distance_avg:.2f}). Brain may be ignored.")]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/quality_monitoring.py` around lines 177 - 178, The alert message in the being_ignored detector currently prints correction densities in the wrong order; change the f-string in the Alert returned in quality_monitoring.py so it shows previous.correction_density before current.correction_density (i.e., f"Corrections dropped {previous.correction_density:.2f} → {current.correction_density:.2f} ...") and keep the rest of the message (including edit_distance_avg from current) unchanged so the arrow correctly indicates the drop.src/gradata/enhancements/router_warmstart.py (1)
37-120:⚠️ Potential issue | 🟠 MajorDB access should use BrainContext parameter.
The function
warm_start_routerdirectly creates a sqlite3 connection (line 74) without accepting aBrainContextparameter. This bypasses any connection pooling, transaction management, or DB abstraction that BrainContext provides and creates inconsistency with other core SDK patterns.Consider refactoring the function signature to accept
BrainContextand use its DB facilities instead of direct sqlite3 access.As per coding guidelines: "all functions accepting BrainContext where DB access occurs" in
src/gradata/**/*.py.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/router_warmstart.py` around lines 37 - 120, The warm_start_router function currently opens sqlite3 directly (sqlite3.connect) which bypasses BrainContext; change warm_start_router to accept a BrainContext parameter (e.g., ctx: BrainContext) instead of db_path, use ctx DB access methods (connection/execute/row fetching or a provided query helper) to read CORRECTION events, remove direct sqlite3 usage, and preserve the existing logic that constructs QLearningRouter (RouterConfig), calls router.route/reward_from_severity/update_reward, and router.load/save; also update any callers to pass a BrainContext and ensure the function uses ctx’s transaction/cleanup semantics rather than manually opening/closing sqlite3 connections.src/gradata/contrib/patterns/guardrails.py (3)
637-648:⚠️ Potential issue | 🟡 MinorRemove dead code and address incomplete guard implementation.
Line 638 contains a dead code statement:
str(data).lower() if data else ""computes a result that is discarded. Additionally, the guard check function always returns"pass"(line 644), making these guards ineffective. The comment on lines 639-641 acknowledges this is a heuristic placeholder.Proposed fix to remove dead code
def _make_check(rule_text: str, rule_cat: str) -> Callable[[Any], GuardCheck]: """Create a check function that scans output for rule violations.""" def check_fn(data: Any) -> GuardCheck: - str(data).lower() if data else "" # Simple keyword check — does the output violate the rule? # Rules are phrased as "never X" or "always Y" # This is a heuristic; real guardrails need LLM-backed checksThe comment indicates this is incomplete. Would you like me to generate an issue to track implementing proper LLM-backed rule validation for graduated rules?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/contrib/patterns/guardrails.py` around lines 637 - 648, In check_fn (the closure returned in this block), remove the dead expression `str(data).lower() if data else ""` and instead compute and reuse a normalized output string (e.g., text = str(data).lower() or "") to perform a real heuristic check: parse the rule_text phrase for simple patterns like "never <term>" or "always <term>" and set GuardCheck.result to "fail" when the heuristic finds a violation (and "pass" otherwise); populate GuardCheck.details with the matched term and a short reason, include rule name using rule_cat (as now), and set action_taken to the concrete action you want for fail vs pass so the guard is not always a no-op. Ensure you reference the existing symbols check_fn, rule_text, rule_cat, and GuardCheck when making changes.
633-633:⚠️ Potential issue | 🟡 MinorRemove dead code statement.
The expression
rule.principle.lower()computes a result that is immediately discarded. This statement has no effect and should be removed.Proposed fix
guards = [] for rule in rules: - rule.principle.lower() - def _make_check(rule_text: str, rule_cat: str) -> Callable[[Any], GuardCheck]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/contrib/patterns/guardrails.py` at line 633, Remove the dead expression `rule.principle.lower()` that computes a value and discards it; either delete that statement entirely or, if the intent was to normalize the principle, assign the lowercased value back (e.g., set `rule.principle = rule.principle.lower()` or call the appropriate normalization method). Update the code in the same scope where `rule.principle.lower()` appears so the normalized value is actually used or the no-op is eliminated.
614-629:⚠️ Potential issue | 🟠 MajorAdd BrainContext parameter for database access.
The function
guards_from_graduated_rules()appears to access the database throughget_rule_context()andctx.for_guardrails()but doesn't accept aBrainContextparameter. As per coding guidelines, all functions performing DB access in the core SDK must accept BrainContext.Proposed signature change
-def guards_from_graduated_rules() -> list[Guard]: +def guards_from_graduated_rules(ctx: BrainContext) -> list[Guard]: """Build Guard objects from graduated SECURITY/ACCURACY rules. Graduated rules in safety-related categories automatically become output guards, so the guardrail set grows from corrections. Example: A SECURITY rule "never include API keys" becomes a Guard that checks output for common API key patterns. + + Args: + ctx: BrainContext for database access. """ try: from gradata.rules.rule_context import get_rule_context except ImportError: return [] - ctx = get_rule_context() - rules = ctx.for_guardrails() + rule_ctx = get_rule_context(ctx) + rules = rule_ctx.for_guardrails()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/contrib/patterns/guardrails.py` around lines 614 - 629, The function guards_from_graduated_rules currently obtains DB access via get_rule_context() and ctx.for_guardrails(), but must accept a BrainContext parameter instead; change the signature of guards_from_graduated_rules to accept a BrainContext (e.g., def guards_from_graduated_rules(brain_ctx: BrainContext) -> list[Guard]), remove the internal import/usage of get_rule_context(), and use the provided brain_ctx to call for_guardrails (or obtain ctx from brain_ctx if needed); update any call sites to pass the BrainContext and adjust imports/types to include BrainContext and maintain the same return behavior from ctx.for_guardrails().src/gradata/_migrations.py (2)
80-92: 🧹 Nitpick | 🔵 TrivialConsider using a context manager for connection safety.
The connection acquired at line 80 is closed at line 92, but if an exception occurs between
conn.commit()(line 91) andconn.close()(line 92), the connection won't be properly released. Consider using a context manager or try/finally block to ensure cleanup.♻️ Proposed fix using context manager pattern
def run_migrations(db_path: str | Path) -> int: """Apply pending migrations. Returns count applied.""" if not Path(db_path).exists(): return 0 conn = get_connection(db_path) - applied = 0 - for sql in _BASE_TABLES: - with contextlib.suppress(sqlite3.OperationalError): - conn.execute(sql) - for sql in _MIGRATIONS: - try: - conn.execute(sql) - applied += 1 - except sqlite3.OperationalError: - pass # Column/index already exists - conn.commit() - conn.close() - return applied + try: + applied = 0 + for sql in _BASE_TABLES: + with contextlib.suppress(sqlite3.OperationalError): + conn.execute(sql) + for sql in _MIGRATIONS: + try: + conn.execute(sql) + applied += 1 + except sqlite3.OperationalError: + pass # Column/index already exists + conn.commit() + return applied + finally: + conn.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/_migrations.py` around lines 80 - 92, The migration code opens a SQLite connection with get_connection(db_path) and manually calls conn.commit() and conn.close(), which can leak the connection if an exception occurs; change the block that iterates over _BASE_TABLES and _MIGRATIONS to use a context manager (e.g., "with get_connection(db_path) as conn:" or wrap in try/finally) so the connection is always closed and commit is done inside the managed scope; ensure the loops over _BASE_TABLES and _MIGRATIONS still execute the same operations (suppressing sqlite3.OperationalError for base tables and counting applied migrations) and that applied counting and conn.commit() occur within the context so cleanup is guaranteed even on errors.
76-93:⚠️ Potential issue | 🟡 MinorAdd
BrainContextparameter torun_migrations()for DB access consistency.The function performs database access via
get_connection()(line 80) but doesn't accept aBrainContextparameter, contrary to the coding guideline requiring all DB-accessing functions to acceptBrainContext. The context is available when the function is called inbrain.py:110, so it should be passed through.Additionally, consider wrapping the connection in a context manager (
with get_connection(...) as conn:) instead of relying on explicitclose()to ensure cleanup if exceptions occur duringexecute()orcommit()operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/_migrations.py` around lines 76 - 93, Change run_migrations to accept a BrainContext parameter (e.g. def run_migrations(ctx: BrainContext) -> int) and use the context's DB path when opening the connection (call get_connection(ctx.db_path) instead of get_connection(db_path)); open the connection with a context manager (with get_connection(ctx.db_path) as conn:) so executes/commit are cleaned up automatically, remove the explicit conn.close(), and update the caller to pass the BrainContext from where run_migrations is invoked.src/gradata/integrations/openai_adapter.py (2)
35-35:⚠️ Potential issue | 🟡 MinorRemove hardcoded default path.
The default parameter
"./brain"violates the "no hardcoded paths" guideline. Consider requiring the caller to explicitly providebrain_diror sourcing it from an environment variable.As per coding guidelines: "no hardcoded paths" for
src/gradata/**/*.py.♻️ Suggested fix
-def patch_openai(client: Any, brain_dir: str | Path = "./brain") -> Any: +def patch_openai(client: Any, brain_dir: str | Path) -> Any:Or use an environment variable:
+import os + +DEFAULT_BRAIN_DIR = os.getenv("GRADATA_BRAIN_DIR", "./brain") + -def patch_openai(client: Any, brain_dir: str | Path = "./brain") -> Any: +def patch_openai(client: Any, brain_dir: str | Path = DEFAULT_BRAIN_DIR) -> Any:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/integrations/openai_adapter.py` at line 35, The patch_openai function currently uses a hardcoded default brain_dir "./brain"; remove that hardcoded default by making brain_dir a required parameter (change signature to patch_openai(client: Any, brain_dir: str | Path)) or alternatively default it from an environment variable (e.g., os.environ.get("GRADATA_BRAIN_DIR")) and validate it is set; update callers to pass brain_dir, add an import for os if using env var, and add a clear error/exception in patch_openai if brain_dir is missing or invalid to prevent silent failures.
87-89: 🧹 Nitpick | 🔵 TrivialAdd bounds check before accessing
choices[0].Accessing
response.choices[0]without verifying thatchoicesis non-empty can raise anIndexError. While the try-except block prevents a crash, it would log an unhelpful "Response capture skipped" message. Add an explicit length check for cleaner error handling.♻️ Suggested refactor
# Capture response for tracking try: - ai_content = response.choices[0].message.content - if ai_content: + if response.choices: + ai_content = response.choices[0].message.content + else: + ai_content = None + + if ai_content: brain.log_output(ai_content, output_type="chat")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/integrations/openai_adapter.py` around lines 87 - 89, The code accesses response.choices[0] without checking that response.choices is non-empty, which can raise IndexError; modify the block handling the OpenAI response in openai_adapter.py to first verify that response.choices exists and has at least one element (e.g., if response.choices and len(response.choices) > 0) before reading response.choices[0].message.content, only then assign ai_content and call brain.log_output(ai_content, output_type="chat"); keep the surrounding try/except but use this explicit bounds check to avoid the vague "Response capture skipped" path and to short-circuit cleanly when no choices are present.src/gradata/_context_compile.py (1)
8-15:⚠️ Potential issue | 🟠 MajorAdd postponed annotations import to satisfy core SDK typing standard.
This module is missing
from __future__ import annotations, which is required forsrc/gradata/**/*.py.As per coding guidelines, "
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/_context_compile.py` around lines 8 - 15, Add the postponed annotations import at the top of the module: insert "from __future__ import annotations" as the first statement (or immediately after any module docstring) in src/gradata/_context_compile.py so the file complies with the core SDK typing standard; this will ensure forward references work for the existing TYPE_CHECKING import and the referenced BrainContext type usage.src/gradata/_tag_taxonomy.py (1)
12-16:⚠️ Potential issue | 🟠 MajorAdd postponed annotations import to meet SDK typing baseline.
This module is missing
from __future__ import annotations, required for core SDK files.As per coding guidelines, "
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/_tag_taxonomy.py` around lines 12 - 16, The module gradata._tag_taxonomy is missing the required typing future import; add the line "from __future__ import annotations" at the top of the file before any other imports so the module meets the SDK typing baseline and enables postponed evaluation of annotations used by functions/classes in this module.src/gradata/_manifest_helpers.py (1)
8-15:⚠️ Potential issue | 🟠 MajorMissing required postponed-annotations import.
Please add
from __future__ import annotationsto align this core SDK module with repo typing standards.As per coding guidelines, "
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/_manifest_helpers.py` around lines 8 - 15, Add the required postponed-annotations import at the top of the gradata._manifest_helpers module by inserting "from __future__ import annotations" as the very first import statement so the file (module gradata._manifest_helpers) uses postponed evaluation of type annotations and conforms to the core SDK typing standard; ensure it appears before any other imports such as re, typing.TYPE_CHECKING, gradata._paths, and gradata._db.get_connection.
♻️ Duplicate comments (3)
src/gradata/hooks/tool_failure_emit.py (1)
17-26: 🧹 Nitpick | 🔵 TrivialConsider making
Exceptionpattern case-insensitive.Line 21 uses
re.compile(r"\bException\b")withoutre.I, so it won't match lowercaseexceptionin logs or error messages.
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/hooks/tool_failure_emit.py` around lines 17 - 26, The regex for matching "Exception" in the ERROR_PATTERNS list is case-sensitive; update the re.compile(r"\bException\b") entry in ERROR_PATTERNS to be case-insensitive (e.g., add the re.I flag or use a case-insensitive inline modifier) so it matches "exception" as well; locate the ERROR_PATTERNS constant and modify the specific re.compile call for "\bException\b" accordingly.src/gradata/hooks/session_close.py (1)
24-39:⚠️ Potential issue | 🟠 MajorUse the locked lessons writer to prevent race conditions.
Line 37 uses
lessons_path.write_text()directly instead of the lockedwrite_lessons_safe()utility. A concurrent correction or self-healing patch at session end could cause data loss.
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/hooks/session_close.py` around lines 24 - 39, The current _run_graduation function writes lessons atomically via lessons_path.write_text(), which can race with other writers; replace that direct write with the project's locked writer utility write_lessons_safe (import it from its module) and call it with the lessons target and formatted content (use format_lessons(active)) so the lessons file is updated under the safe/locked write path instead of lessons_path.write_text().src/gradata/hooks/agent_graduation.py (1)
24-51: 🧹 Nitpick | 🔵 TrivialConsider adding debug logging for silent failures.
Per hook guidelines, errors must be silent (which this achieves), but completely silent failures make debugging difficult. Consider logging at debug level.
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/hooks/agent_graduation.py` around lines 24 - 51, The hook currently swallows all exceptions in main (and thus loses error context); add debug-level logging to capture exceptions: import logging and create a module logger (e.g., logger = logging.getLogger(__name__)), then inside the except block of main (and optionally where resolve_brain_dir() returns falsy) call logger.debug with a clear message and exc_info=True so the traceback and exception are recorded; reference the main function, resolve_brain_dir(), _infer_agent_type(), BrainContext.from_brain_dir, and emit to indicate where failures should be logged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gradata/_context_compile.py`:
- Around line 100-101: The except block in src/gradata/_context_compile.py
currently returns the raw exception string (return f"[context compile error:
{e}]") which can leak internals; instead catch the exception, log the full
exception details internally (e.g., logger.exception or logging.exception) for
diagnostics, and return a generic non-sensitive message such as "[context
compile error]" (or an empty/neutral string) to the caller; add or reuse a
module logger via logging.getLogger(__name__) if needed and remove any exposure
of the exception object from the returned value.
In `@src/gradata/contrib/patterns/tree_of_thoughts.py`:
- Around line 139-143: The "Never ..." candidate is being created with a global
replace that can remove internal "always " tokens; instead compute a single
normalized base string from lesson_description (e.g., base =
lesson_description.lower().removeprefix('always ').strip()) and reuse that base
for both the "Always ..." and "Never ..." candidates so you build candidates as
lesson_description, f"Always {base}", and f"Never {base}" (use the same
normalization logic referenced in the existing second candidate).
In `@src/gradata/enhancements/behavioral_extractor.py`:
- Around line 204-205: The list comprehension using zip(final_sents,
final_sent_sets, strict=True) can raise ValueError on a length mismatch; replace
the use of zip(..., strict=True) with a normal zip and add an explicit length
check (e.g., if len(final_sents) != len(final_sent_sets): raise
ValueError("final_sents and final_sent_sets length mismatch") ) before the
comprehension to produce a clearer, controlled error message; make the same
change for the other occurrences of zip(..., strict=True) that pair draft/final
sentence lists (referencing the variables final_sents, final_sent_sets,
draft_sent_sets and the _sentence_overlap usage) so all mismatches are detected
with a clear assertion instead of the zip strict ValueError.
In `@src/gradata/enhancements/meta_rules_storage.py`:
- Around line 70-72: Replace the broad
contextlib.suppress(sqlite3.OperationalError) used when executing
schema-altering statements (the loop that runs _ADD_CONTEXT_WEIGHTS_SQL,
_ADD_APPLIES_WHEN_SQL, _ADD_NEVER_WHEN_SQL, _ADD_TRANSFER_SCOPE_SQL) with a
try/except that catches sqlite3.OperationalError as e and re-raises unless the
error message indicates a duplicate column (e.g., contains "duplicate column" or
"duplicate column name", case-insensitive); do the same replacement for the
analogous super_meta_rules migration loop (the statements executed around the
second suppression block) so only duplicate-column errors are ignored and all
other OperationalError cases propagate.
In `@src/gradata/enhancements/rule_evolution.py`:
- Around line 281-284: Compute _detect_opposite_direction(new_desc,
best_rule.description) once into a local variable (e.g., opposite) before the
two if checks and then use that variable in the conditions instead of calling
_detect_opposite_direction twice; update the branching that returns
(RuleRelation.UPDATES, best_rule) and (RuleRelation.EXTENDS, best_rule) to
reference the new variable while retaining the same comparisons against
best_similarity, update_threshold, and extend_threshold (ensure you only
evaluate opposite when best_rule is not None).
In `@src/gradata/hooks/brain_maintain.py`:
- Around line 51-64: The main function currently returns None unconditionally;
update main (and its exit paths) to return a meaningful status dict on success
(e.g., {"result":"maintenance complete"}) while preserving the existing
early-return when resolve_brain_dir() yields falsy; keep exception handling but
return an error status on failure or rethrow if preferred. Specifically, in
function main (uses resolve_brain_dir(), BrainContext.from_brain_dir,
_rebuild_fts, _generate_manifest) change the final return to a success dict
after _generate_manifest completes, and ensure the early branch that returns
when not brain_dir still returns an explicit status (e.g., {"result":"no brain
dir"}) and the except block returns or logs an error status (e.g.,
{"error":"..."}).
In `@src/gradata/hooks/context_inject.py`:
- Around line 44-46: The loop handling search results assumes each result r is a
dict and uses r.get(), which fails for object results; update the extraction to
try object attributes first (getattr(r, "text", None) or getattr(r, "content",
None)), then fall back to dict access (r.get("text") or r.get("content")) and
finally str(r) so both dicts and objects from brain.search() are handled; modify
the snippet around the for r in results loop in context_inject.py to use this
attribute-or-key fallback for text/snippet extraction.
In `@src/gradata/hooks/inject_brain_rules.py`:
- Around line 33-58: Wrap the file I/O and parsing steps in a broad try/except
inside main so any exceptions from lessons_path.read_text, parse_lessons, or
related operations are caught and the hook returns None (silent failure) instead
of raising; specifically, around the block that calls resolve_brain_dir(), reads
lessons.md via lessons_path.read_text(), calls parse_lessons(text), and
processes results (including filtered/scored/lines/block), catch Exception and
return None. Keep existing early-return checks (resolve_brain_dir and is_file)
but ensure unexpected errors during I/O or parsing do not propagate.
In `@src/gradata/hooks/pre_compact.py`:
- Line 68: The MD5 call used to generate dir_hash is incompatible with
FIPS-enabled Python because it omits the usedforsecurity flag; update the call
that constructs dir_hash in pre_compact.py (the hashlib.md5(...) that encodes
str(brain_dir)) to pass usedforsecurity=False so the non-cryptographic
filename-uniqueness hash works on FIPS systems, keeping the rest of the logic
(encoding and hexdigest slicing) unchanged.
In `@src/gradata/hooks/rule_enforcement.py`:
- Around line 24-50: Wrap the file I/O and parsing steps in main with exception
handling so no exceptions escape the hook: around resolve_brain_dir(),
lessons_path.read_text(...), and the parse_lessons(text) call (and the loop that
builds rules using MAX_REMINDERS), catch broad exceptions, optionally log at
debug if a logger exists, and return None on error; ensure parse_lessons being
None is still handled as before and that the function never lets file or parsing
errors propagate.
In `@src/gradata/hooks/secret_scan.py`:
- Around line 44-73: The main function lacks defensive error handling; wrap the
body of main (which builds contents_to_scan, iterates edits, calls
_scan_content, and assembles the findings/return value) in a try/except that
catches Exception and returns None on error to match other hooks (e.g.,
implicit_feedback.py, context_inject.py). Ensure the try block includes all
logic that reads tool_input, constructs contents_to_scan, calls _scan_content,
and builds the final decision payload, and that the except logs or swallows the
exception and returns None so failures in _scan_content or unexpected input
don't crash the hook.
In `@src/gradata/hooks/session_persist.py`:
- Around line 68-93: Remove the redundant try/except in the main function of
session_persist.py: delete the surrounding try/except block so main(data: dict)
executes its logic directly and lets run_hook's wrapper (run_hook in _base.py)
handle exceptions; ensure the function still returns None at the end and keeps
all existing behavior (resolve_brain_dir(), _get_session_number(),
_get_modified_files(), file write to persist_dir) unchanged.
In `@src/gradata/integrations/openai_adapter.py`:
- Around line 98-99: The patched client currently stores Brain on client._brain
and calls brain.log_output() and brain.observe() inside patched_create, which is
unsafe for multi-threaded use; fix by adding a threading.RLock (e.g.,
client._brain_lock = threading.RLock()) when you attach the brain and wrap all
Brain accesses in patched_create (acquire the lock around calls to
brain.log_output() and brain.observe()) to serialize SQLite writes;
alternatively, if you prefer not to add locking, update the module/class
docstring and add a clear comment where client._brain is set stating the patched
client MUST NOT be shared across threads and recommend one Brain per process or
using process-level locks (reference symbols: Brain, patched_create,
client._brain, client.chat.completions.create, brain.log_output(),
brain.observe()).
---
Outside diff comments:
In `@src/gradata/__init__.py`:
- Around line 1-26: Add the postponed-annotations future import at the very top
of the module: insert from __future__ import annotations as the first statement
in this file (before the module docstring and any other imports) so the typing
rule is satisfied for the core SDK; keep the rest of the file (the module
docstring, __version__, and the logging/os imports) unchanged.
In `@src/gradata/_context_compile.py`:
- Around line 8-15: Add the postponed annotations import at the top of the
module: insert "from __future__ import annotations" as the first statement (or
immediately after any module docstring) in src/gradata/_context_compile.py so
the file complies with the core SDK typing standard; this will ensure forward
references work for the existing TYPE_CHECKING import and the referenced
BrainContext type usage.
In `@src/gradata/_doctor.py`:
- Around line 246-264: The print_diagnosis function in src/gradata/_doctor.py
currently calls print() for all output; replace these prints with structured
logging (e.g., get a module logger via logging.getLogger(__name__) and emit
logger.info for normal lines, logger.warning for "degraded" messages and
logger.error for the failing status) or alternatively return the formatted
multi-line string from print_diagnosis and let the CLI layer print it; update
the loop over report["checks"] to log each check with
icons.get(check["status"]), the humanized name and detail instead of printing,
and ensure the final status block uses the appropriate logger level (healthy →
info, degraded → warning, else → error) so no print() calls remain in the
module.
In `@src/gradata/_manifest_helpers.py`:
- Around line 8-15: Add the required postponed-annotations import at the top of
the gradata._manifest_helpers module by inserting "from __future__ import
annotations" as the very first import statement so the file (module
gradata._manifest_helpers) uses postponed evaluation of type annotations and
conforms to the core SDK typing standard; ensure it appears before any other
imports such as re, typing.TYPE_CHECKING, gradata._paths, and
gradata._db.get_connection.
In `@src/gradata/_migrations.py`:
- Around line 80-92: The migration code opens a SQLite connection with
get_connection(db_path) and manually calls conn.commit() and conn.close(), which
can leak the connection if an exception occurs; change the block that iterates
over _BASE_TABLES and _MIGRATIONS to use a context manager (e.g., "with
get_connection(db_path) as conn:" or wrap in try/finally) so the connection is
always closed and commit is done inside the managed scope; ensure the loops over
_BASE_TABLES and _MIGRATIONS still execute the same operations (suppressing
sqlite3.OperationalError for base tables and counting applied migrations) and
that applied counting and conn.commit() occur within the context so cleanup is
guaranteed even on errors.
- Around line 76-93: Change run_migrations to accept a BrainContext parameter
(e.g. def run_migrations(ctx: BrainContext) -> int) and use the context's DB
path when opening the connection (call get_connection(ctx.db_path) instead of
get_connection(db_path)); open the connection with a context manager (with
get_connection(ctx.db_path) as conn:) so executes/commit are cleaned up
automatically, remove the explicit conn.close(), and update the caller to pass
the BrainContext from where run_migrations is invoked.
In `@src/gradata/_tag_taxonomy.py`:
- Around line 12-16: The module gradata._tag_taxonomy is missing the required
typing future import; add the line "from __future__ import annotations" at the
top of the file before any other imports so the module meets the SDK typing
baseline and enables postponed evaluation of annotations used by
functions/classes in this module.
In `@src/gradata/contrib/enhancements/install_manifest.py`:
- Line 499: Move the function-local "import time" into the module-level imports
in install_manifest.py: add time to the existing top-of-file imports alongside
the other standard-library imports and remove the in-function "import time"
statement so the module uses the top-level time import throughout (locate the
in-function import in the function where "import time" currently appears and
delete it after adding the module-level import).
In `@src/gradata/contrib/patterns/guardrails.py`:
- Around line 637-648: In check_fn (the closure returned in this block), remove
the dead expression `str(data).lower() if data else ""` and instead compute and
reuse a normalized output string (e.g., text = str(data).lower() or "") to
perform a real heuristic check: parse the rule_text phrase for simple patterns
like "never <term>" or "always <term>" and set GuardCheck.result to "fail" when
the heuristic finds a violation (and "pass" otherwise); populate
GuardCheck.details with the matched term and a short reason, include rule name
using rule_cat (as now), and set action_taken to the concrete action you want
for fail vs pass so the guard is not always a no-op. Ensure you reference the
existing symbols check_fn, rule_text, rule_cat, and GuardCheck when making
changes.
- Line 633: Remove the dead expression `rule.principle.lower()` that computes a
value and discards it; either delete that statement entirely or, if the intent
was to normalize the principle, assign the lowercased value back (e.g., set
`rule.principle = rule.principle.lower()` or call the appropriate normalization
method). Update the code in the same scope where `rule.principle.lower()`
appears so the normalized value is actually used or the no-op is eliminated.
- Around line 614-629: The function guards_from_graduated_rules currently
obtains DB access via get_rule_context() and ctx.for_guardrails(), but must
accept a BrainContext parameter instead; change the signature of
guards_from_graduated_rules to accept a BrainContext (e.g., def
guards_from_graduated_rules(brain_ctx: BrainContext) -> list[Guard]), remove the
internal import/usage of get_rule_context(), and use the provided brain_ctx to
call for_guardrails (or obtain ctx from brain_ctx if needed); update any call
sites to pass the BrainContext and adjust imports/types to include BrainContext
and maintain the same return behavior from ctx.for_guardrails().
In `@src/gradata/enhancements/git_backfill.py`:
- Line 190: The parameter brain currently typed as Any should use a stricter
type: define a Protocol (e.g., BrainLike) that declares the methods used (at
least correct(self, draft: str, final: str, session: int | None = None) ->
dict[str, Any]) or, if a concrete Brain class exists, use a forward reference to
that class; then update the function/method signature that contains "brain: Any"
in git_backfill.py to use BrainLike or "Brain" instead and import
typing.Protocol and typing.Any as needed so type checkers can validate usages of
brain.correct and related calls.
In `@src/gradata/enhancements/quality_monitoring.py`:
- Around line 177-178: The alert message in the being_ignored detector currently
prints correction densities in the wrong order; change the f-string in the Alert
returned in quality_monitoring.py so it shows previous.correction_density before
current.correction_density (i.e., f"Corrections dropped
{previous.correction_density:.2f} → {current.correction_density:.2f} ...") and
keep the rest of the message (including edit_distance_avg from current)
unchanged so the arrow correctly indicates the drop.
In `@src/gradata/enhancements/router_warmstart.py`:
- Around line 37-120: The warm_start_router function currently opens sqlite3
directly (sqlite3.connect) which bypasses BrainContext; change warm_start_router
to accept a BrainContext parameter (e.g., ctx: BrainContext) instead of db_path,
use ctx DB access methods (connection/execute/row fetching or a provided query
helper) to read CORRECTION events, remove direct sqlite3 usage, and preserve the
existing logic that constructs QLearningRouter (RouterConfig), calls
router.route/reward_from_severity/update_reward, and router.load/save; also
update any callers to pass a BrainContext and ensure the function uses ctx’s
transaction/cleanup semantics rather than manually opening/closing sqlite3
connections.
In `@src/gradata/integrations/openai_adapter.py`:
- Line 35: The patch_openai function currently uses a hardcoded default
brain_dir "./brain"; remove that hardcoded default by making brain_dir a
required parameter (change signature to patch_openai(client: Any, brain_dir: str
| Path)) or alternatively default it from an environment variable (e.g.,
os.environ.get("GRADATA_BRAIN_DIR")) and validate it is set; update callers to
pass brain_dir, add an import for os if using env var, and add a clear
error/exception in patch_openai if brain_dir is missing or invalid to prevent
silent failures.
- Around line 87-89: The code accesses response.choices[0] without checking that
response.choices is non-empty, which can raise IndexError; modify the block
handling the OpenAI response in openai_adapter.py to first verify that
response.choices exists and has at least one element (e.g., if response.choices
and len(response.choices) > 0) before reading
response.choices[0].message.content, only then assign ai_content and call
brain.log_output(ai_content, output_type="chat"); keep the surrounding
try/except but use this explicit bounds check to avoid the vague "Response
capture skipped" path and to short-circuit cleanly when no choices are present.
---
Duplicate comments:
In `@src/gradata/hooks/agent_graduation.py`:
- Around line 24-51: The hook currently swallows all exceptions in main (and
thus loses error context); add debug-level logging to capture exceptions: import
logging and create a module logger (e.g., logger = logging.getLogger(__name__)),
then inside the except block of main (and optionally where resolve_brain_dir()
returns falsy) call logger.debug with a clear message and exc_info=True so the
traceback and exception are recorded; reference the main function,
resolve_brain_dir(), _infer_agent_type(), BrainContext.from_brain_dir, and emit
to indicate where failures should be logged.
In `@src/gradata/hooks/session_close.py`:
- Around line 24-39: The current _run_graduation function writes lessons
atomically via lessons_path.write_text(), which can race with other writers;
replace that direct write with the project's locked writer utility
write_lessons_safe (import it from its module) and call it with the lessons
target and formatted content (use format_lessons(active)) so the lessons file is
updated under the safe/locked write path instead of lessons_path.write_text().
In `@src/gradata/hooks/tool_failure_emit.py`:
- Around line 17-26: The regex for matching "Exception" in the ERROR_PATTERNS
list is case-sensitive; update the re.compile(r"\bException\b") entry in
ERROR_PATTERNS to be case-insensitive (e.g., add the re.I flag or use a
case-insensitive inline modifier) so it matches "exception" as well; locate the
ERROR_PATTERNS constant and modify the specific re.compile call for
"\bException\b" accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 89857ad5-8965-49c2-9f3c-73e37c6ff0bc
📒 Files selected for processing (107)
pyproject.tomlsrc/gradata/__init__.pysrc/gradata/_brain_manifest.pysrc/gradata/_context_compile.pysrc/gradata/_context_packet.pysrc/gradata/_core.pysrc/gradata/_db.pysrc/gradata/_doctor.pysrc/gradata/_embed.pysrc/gradata/_encryption.pysrc/gradata/_fact_extractor.pysrc/gradata/_installer.pysrc/gradata/_manifest_helpers.pysrc/gradata/_manifest_quality.pysrc/gradata/_math.pysrc/gradata/_migrations.pysrc/gradata/_paths.pysrc/gradata/_query.pysrc/gradata/_stats.pysrc/gradata/_tag_taxonomy.pysrc/gradata/_validator.pysrc/gradata/audit.pysrc/gradata/benchmarks/swe_bench.pysrc/gradata/brain.pysrc/gradata/brain_inspection.pysrc/gradata/cli.pysrc/gradata/context_wrapper.pysrc/gradata/contrib/enhancements/eval_benchmark.pysrc/gradata/contrib/enhancements/install_manifest.pysrc/gradata/contrib/enhancements/quality_gates.pysrc/gradata/contrib/patterns/context_brackets.pysrc/gradata/contrib/patterns/evaluator.pysrc/gradata/contrib/patterns/execute_qualify.pysrc/gradata/contrib/patterns/guardrails.pysrc/gradata/contrib/patterns/human_loop.pysrc/gradata/contrib/patterns/loop_detection.pysrc/gradata/contrib/patterns/middleware.pysrc/gradata/contrib/patterns/orchestrator.pysrc/gradata/contrib/patterns/parallel.pysrc/gradata/contrib/patterns/pipeline.pysrc/gradata/contrib/patterns/q_learning_router.pysrc/gradata/contrib/patterns/reconciliation.pysrc/gradata/contrib/patterns/reflection.pysrc/gradata/contrib/patterns/task_escalation.pysrc/gradata/contrib/patterns/tree_of_thoughts.pysrc/gradata/correction_detector.pysrc/gradata/daemon.pysrc/gradata/detection/__init__.pysrc/gradata/detection/addition_pattern.pysrc/gradata/detection/correction_conflict.pysrc/gradata/detection/mode_classifier.pysrc/gradata/enhancements/behavioral_extractor.pysrc/gradata/enhancements/cluster_manager.pysrc/gradata/enhancements/edit_classifier.pysrc/gradata/enhancements/git_backfill.pysrc/gradata/enhancements/instruction_cache.pysrc/gradata/enhancements/lesson_discriminator.pysrc/gradata/enhancements/memory_taxonomy.pysrc/gradata/enhancements/meta_rules.pysrc/gradata/enhancements/meta_rules_storage.pysrc/gradata/enhancements/observation_hooks.pysrc/gradata/enhancements/pattern_extractor.pysrc/gradata/enhancements/pattern_integration.pysrc/gradata/enhancements/quality_monitoring.pysrc/gradata/enhancements/reporting.pysrc/gradata/enhancements/router_warmstart.pysrc/gradata/enhancements/rule_evolution.pysrc/gradata/enhancements/rule_verifier.pysrc/gradata/enhancements/self_improvement.pysrc/gradata/enhancements/similarity.pysrc/gradata/enhancements/super_meta_rules.pysrc/gradata/events_bus.pysrc/gradata/graph.pysrc/gradata/hooks/_base.pysrc/gradata/hooks/agent_graduation.pysrc/gradata/hooks/agent_precontext.pysrc/gradata/hooks/auto_correct.pysrc/gradata/hooks/brain_maintain.pysrc/gradata/hooks/claude_code.pysrc/gradata/hooks/config_protection.pysrc/gradata/hooks/context_inject.pysrc/gradata/hooks/implicit_feedback.pysrc/gradata/hooks/inject_brain_rules.pysrc/gradata/hooks/pre_compact.pysrc/gradata/hooks/rule_enforcement.pysrc/gradata/hooks/secret_scan.pysrc/gradata/hooks/session_close.pysrc/gradata/hooks/session_persist.pysrc/gradata/hooks/tool_failure_emit.pysrc/gradata/inspection.pysrc/gradata/integrations/anthropic_adapter.pysrc/gradata/integrations/crewai_adapter.pysrc/gradata/integrations/embeddings.pysrc/gradata/integrations/langchain_adapter.pysrc/gradata/integrations/openai_adapter.pysrc/gradata/mcp_server.pysrc/gradata/mcp_tools.pysrc/gradata/onboard.pysrc/gradata/rules/__init__.pysrc/gradata/rules/rule_engine.pysrc/gradata/rules/rule_graph.pysrc/gradata/rules/rule_tracker.pysrc/gradata/rules/scope.pysrc/gradata/security/__init__.pysrc/gradata/security/correction_provenance.pysrc/gradata/security/manifest_signing.pysrc/gradata/sidecar/watcher.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
src/gradata/**/*.py
⚙️ CodeRabbit configuration file
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required), no print()
statements (use logging), all functions accepting BrainContext where DB access occurs, no hardcoded paths. Severity
scoring must clamp to [0,1]. Confidence values must be in [0.0, 1.0].
Files:
src/gradata/_embed.pysrc/gradata/contrib/patterns/reflection.pysrc/gradata/integrations/anthropic_adapter.pysrc/gradata/_math.pysrc/gradata/_brain_manifest.pysrc/gradata/rules/rule_graph.pysrc/gradata/mcp_tools.pysrc/gradata/_encryption.pysrc/gradata/integrations/langchain_adapter.pysrc/gradata/contrib/patterns/reconciliation.pysrc/gradata/enhancements/quality_monitoring.pysrc/gradata/enhancements/pattern_integration.pysrc/gradata/contrib/enhancements/install_manifest.pysrc/gradata/integrations/openai_adapter.pysrc/gradata/contrib/enhancements/eval_benchmark.pysrc/gradata/detection/addition_pattern.pysrc/gradata/enhancements/super_meta_rules.pysrc/gradata/enhancements/router_warmstart.pysrc/gradata/enhancements/lesson_discriminator.pysrc/gradata/contrib/patterns/guardrails.pysrc/gradata/security/correction_provenance.pysrc/gradata/_db.pysrc/gradata/_paths.pysrc/gradata/_installer.pysrc/gradata/contrib/patterns/pipeline.pysrc/gradata/integrations/embeddings.pysrc/gradata/integrations/crewai_adapter.pysrc/gradata/security/manifest_signing.pysrc/gradata/_query.pysrc/gradata/contrib/patterns/task_escalation.pysrc/gradata/detection/__init__.pysrc/gradata/contrib/patterns/context_brackets.pysrc/gradata/sidecar/watcher.pysrc/gradata/_context_compile.pysrc/gradata/_validator.pysrc/gradata/rules/scope.pysrc/gradata/correction_detector.pysrc/gradata/security/__init__.pysrc/gradata/enhancements/edit_classifier.pysrc/gradata/enhancements/rule_verifier.pysrc/gradata/contrib/patterns/execute_qualify.pysrc/gradata/enhancements/git_backfill.pysrc/gradata/contrib/patterns/evaluator.pysrc/gradata/contrib/patterns/loop_detection.pysrc/gradata/graph.pysrc/gradata/onboard.pysrc/gradata/enhancements/instruction_cache.pysrc/gradata/enhancements/meta_rules.pysrc/gradata/contrib/patterns/parallel.pysrc/gradata/mcp_server.pysrc/gradata/_stats.pysrc/gradata/_doctor.pysrc/gradata/enhancements/reporting.pysrc/gradata/enhancements/memory_taxonomy.pysrc/gradata/_manifest_helpers.pysrc/gradata/inspection.pysrc/gradata/contrib/patterns/middleware.pysrc/gradata/rules/rule_engine.pysrc/gradata/_migrations.pysrc/gradata/enhancements/similarity.pysrc/gradata/benchmarks/swe_bench.pysrc/gradata/contrib/patterns/orchestrator.pysrc/gradata/daemon.pysrc/gradata/enhancements/pattern_extractor.pysrc/gradata/contrib/patterns/human_loop.pysrc/gradata/contrib/patterns/tree_of_thoughts.pysrc/gradata/enhancements/observation_hooks.pysrc/gradata/enhancements/rule_evolution.pysrc/gradata/rules/rule_tracker.pysrc/gradata/enhancements/self_improvement.pysrc/gradata/_fact_extractor.pysrc/gradata/detection/correction_conflict.pysrc/gradata/__init__.pysrc/gradata/enhancements/meta_rules_storage.pysrc/gradata/context_wrapper.pysrc/gradata/hooks/config_protection.pysrc/gradata/rules/__init__.pysrc/gradata/hooks/claude_code.pysrc/gradata/_core.pysrc/gradata/hooks/auto_correct.pysrc/gradata/_context_packet.pysrc/gradata/hooks/tool_failure_emit.pysrc/gradata/enhancements/cluster_manager.pysrc/gradata/hooks/inject_brain_rules.pysrc/gradata/hooks/session_persist.pysrc/gradata/detection/mode_classifier.pysrc/gradata/hooks/rule_enforcement.pysrc/gradata/brain_inspection.pysrc/gradata/hooks/secret_scan.pysrc/gradata/hooks/pre_compact.pysrc/gradata/hooks/agent_graduation.pysrc/gradata/hooks/implicit_feedback.pysrc/gradata/enhancements/behavioral_extractor.pysrc/gradata/hooks/context_inject.pysrc/gradata/hooks/brain_maintain.pysrc/gradata/cli.pysrc/gradata/hooks/_base.pysrc/gradata/audit.pysrc/gradata/hooks/agent_precontext.pysrc/gradata/contrib/patterns/q_learning_router.pysrc/gradata/contrib/enhancements/quality_gates.pysrc/gradata/_tag_taxonomy.pysrc/gradata/events_bus.pysrc/gradata/brain.pysrc/gradata/_manifest_quality.pysrc/gradata/hooks/session_close.py
src/gradata/integrations/**
⚙️ CodeRabbit configuration file
src/gradata/integrations/**: Framework integrations and nervous system modules. Check for: SSRF protection on any URL from env vars,
thread safety on shared state, embedding cache must be bounded, event bus handlers must never raise.
Files:
src/gradata/integrations/anthropic_adapter.pysrc/gradata/integrations/langchain_adapter.pysrc/gradata/integrations/openai_adapter.pysrc/gradata/integrations/embeddings.pysrc/gradata/integrations/crewai_adapter.py
src/gradata/hooks/**
⚙️ CodeRabbit configuration file
src/gradata/hooks/**: JavaScript hooks for Claude Code integration. Check for: no shell injection (no execSync with user
input), temp files must use per-user subdirectory, HTTP calls must have timeouts, errors must be silent (never block
the tool chain).
Files:
src/gradata/hooks/config_protection.pysrc/gradata/hooks/claude_code.pysrc/gradata/hooks/auto_correct.pysrc/gradata/hooks/tool_failure_emit.pysrc/gradata/hooks/inject_brain_rules.pysrc/gradata/hooks/session_persist.pysrc/gradata/hooks/rule_enforcement.pysrc/gradata/hooks/secret_scan.pysrc/gradata/hooks/pre_compact.pysrc/gradata/hooks/agent_graduation.pysrc/gradata/hooks/implicit_feedback.pysrc/gradata/hooks/context_inject.pysrc/gradata/hooks/brain_maintain.pysrc/gradata/hooks/_base.pysrc/gradata/hooks/agent_precontext.pysrc/gradata/hooks/session_close.py
| except Exception as e: | ||
| return f"[context compile error: {e}]" | ||
| return f"[context compile error: {e}]" |
There was a problem hiding this comment.
Avoid returning raw exception details from context compilation.
Line 101 returns the raw exception string, which can expose internal implementation details in user-facing context output.
Suggested hardening
+import logging
+
+_log = logging.getLogger("gradata.context_compile")
...
- except Exception as e:
- return f"[context compile error: {e}]"
+ except Exception as e:
+ _log.debug("compile_context failed: %s", e)
+ return "[context compile error]"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/_context_compile.py` around lines 100 - 101, The except block in
src/gradata/_context_compile.py currently returns the raw exception string
(return f"[context compile error: {e}]") which can leak internals; instead catch
the exception, log the full exception details internally (e.g., logger.exception
or logging.exception) for diagnostics, and return a generic non-sensitive
message such as "[context compile error]" (or an empty/neutral string) to the
caller; add or reuse a module logger via logging.getLogger(__name__) if needed
and remove any exposure of the exception object from the returned value.
| candidates = [ | ||
| lesson_description, | ||
| f"Always {lesson_description.lower().lstrip('always ')}", | ||
| f"Always {lesson_description.lower().removeprefix('always ')}", | ||
| f"Never {lesson_description.lower().replace('always ', '').replace('use ', 'use ').strip()}", | ||
| ] |
There was a problem hiding this comment.
Normalize the “Never …” candidate with prefix logic, not global replace.
Line 142 can still remove internal "always " tokens, which can distort candidate meaning. Reuse the same prefix-normalized base used for Line 141.
💡 Proposed fix
- candidates = [
- lesson_description,
- f"Always {lesson_description.lower().removeprefix('always ')}",
- f"Never {lesson_description.lower().replace('always ', '').replace('use ', 'use ').strip()}",
- ]
+ normalized = lesson_description.lower().removeprefix("always ").strip()
+ candidates = [
+ lesson_description,
+ f"Always {normalized}",
+ f"Never {normalized}",
+ ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/contrib/patterns/tree_of_thoughts.py` around lines 139 - 143, The
"Never ..." candidate is being created with a global replace that can remove
internal "always " tokens; instead compute a single normalized base string from
lesson_description (e.g., base = lesson_description.lower().removeprefix('always
').strip()) and reuse that base for both the "Always ..." and "Never ..."
candidates so you build candidates as lesson_description, f"Always {base}", and
f"Never {base}" (use the same normalization logic referenced in the existing
second candidate).
| added_sents = [s for s, ws in zip(final_sents, final_sent_sets, strict=True) | ||
| if not any(_sentence_overlap(ws, ds) > 0.5 for ds in draft_sent_sets)] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
strict=True can raise ValueError on length mismatch.
If final_sents and final_sent_sets have different lengths due to a bug elsewhere, this will raise ValueError at runtime. While this is defensive and helps catch bugs early, ensure upstream code maintains the invariant that these lists are always equal length.
The same pattern appears at lines 251 and 289.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/enhancements/behavioral_extractor.py` around lines 204 - 205, The
list comprehension using zip(final_sents, final_sent_sets, strict=True) can
raise ValueError on a length mismatch; replace the use of zip(..., strict=True)
with a normal zip and add an explicit length check (e.g., if len(final_sents) !=
len(final_sent_sets): raise ValueError("final_sents and final_sent_sets length
mismatch") ) before the comprehension to produce a clearer, controlled error
message; make the same change for the other occurrences of zip(..., strict=True)
that pair draft/final sentence lists (referencing the variables final_sents,
final_sent_sets, draft_sent_sets and the _sentence_overlap usage) so all
mismatches are detected with a clear assertion instead of the zip strict
ValueError.
| for stmt in (_ADD_CONTEXT_WEIGHTS_SQL, _ADD_APPLIES_WHEN_SQL, _ADD_NEVER_WHEN_SQL, _ADD_TRANSFER_SCOPE_SQL): | ||
| try: | ||
| with contextlib.suppress(sqlite3.OperationalError): | ||
| conn.execute(stmt) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all broad sqlite3 OperationalError suppressions in migration paths.
rg -n -C2 'contextlib\.suppress\(sqlite3\.OperationalError\)|ALTER TABLE .* ADD COLUMN|conn\.execute\(stmt\)' src/gradata/enhancements/meta_rules_storage.pyRepository: Gradata/gradata
Length of output: 1511
🏁 Script executed:
cat -n src/gradata/enhancements/meta_rules_storage.py | head -100Repository: Gradata/gradata
Length of output: 3891
🏁 Script executed:
cat -n src/gradata/enhancements/meta_rules_storage.py | sed -n '200,250p'Repository: Gradata/gradata
Length of output: 2066
🏁 Script executed:
rg -n 'contextlib\.suppress' src/gradata/enhancements/meta_rules_storage.pyRepository: Gradata/gradata
Length of output: 193
Silence only "duplicate column" errors during schema migration; re-raise other sqlite3.OperationalError exceptions.
Lines 71 and 225 suppress all sqlite3.OperationalError exceptions, which hides legitimate migration failures—locked databases, corrupt tables, invalid SQL state—and can leave schema partially upgraded. Only "duplicate column" should be silently ignored during backward-compatible migrations.
Proposed fix
for stmt in (_ADD_CONTEXT_WEIGHTS_SQL, _ADD_APPLIES_WHEN_SQL, _ADD_NEVER_WHEN_SQL, _ADD_TRANSFER_SCOPE_SQL):
- with contextlib.suppress(sqlite3.OperationalError):
- conn.execute(stmt)
+ try:
+ conn.execute(stmt)
+ except sqlite3.OperationalError as exc:
+ # Keep backward-compatible "already exists" behavior only.
+ if "duplicate column name" not in str(exc).lower():
+ raiseApply the same change to lines 224–226 (super_meta_rules migration loop).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/enhancements/meta_rules_storage.py` around lines 70 - 72, Replace
the broad contextlib.suppress(sqlite3.OperationalError) used when executing
schema-altering statements (the loop that runs _ADD_CONTEXT_WEIGHTS_SQL,
_ADD_APPLIES_WHEN_SQL, _ADD_NEVER_WHEN_SQL, _ADD_TRANSFER_SCOPE_SQL) with a
try/except that catches sqlite3.OperationalError as e and re-raises unless the
error message indicates a duplicate column (e.g., contains "duplicate column" or
"duplicate column name", case-insensitive); do the same replacement for the
analogous super_meta_rules migration loop (the statements executed around the
second suppression block) so only duplicate-column errors are ignored and all
other OperationalError cases propagate.
| if best_rule is not None and best_similarity > update_threshold and _detect_opposite_direction(new_desc, best_rule.description): | ||
| return (RuleRelation.UPDATES, best_rule) | ||
| if best_rule is not None and best_similarity > extend_threshold and not _detect_opposite_direction(new_desc, best_rule.description): | ||
| return (RuleRelation.EXTENDS, best_rule) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Compute opposite-direction once for clearer branching.
This avoids duplicate work and keeps the branch condition easier to audit.
♻️ Proposed refactor
- if best_rule is not None and best_similarity > update_threshold and _detect_opposite_direction(new_desc, best_rule.description):
+ is_opposite = (
+ _detect_opposite_direction(new_desc, best_rule.description)
+ if best_rule is not None else False
+ )
+ if best_rule is not None and best_similarity > update_threshold and is_opposite:
return (RuleRelation.UPDATES, best_rule)
- if best_rule is not None and best_similarity > extend_threshold and not _detect_opposite_direction(new_desc, best_rule.description):
+ if best_rule is not None and best_similarity > extend_threshold and not is_opposite:
return (RuleRelation.EXTENDS, best_rule)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/enhancements/rule_evolution.py` around lines 281 - 284, Compute
_detect_opposite_direction(new_desc, best_rule.description) once into a local
variable (e.g., opposite) before the two if checks and then use that variable in
the conditions instead of calling _detect_opposite_direction twice; update the
branching that returns (RuleRelation.UPDATES, best_rule) and
(RuleRelation.EXTENDS, best_rule) to reference the new variable while retaining
the same comparisons against best_similarity, update_threshold, and
extend_threshold (ensure you only evaluate opposite when best_rule is not None).
| uid = os.getuid() if hasattr(os, "getuid") else "win" | ||
| user_tmp = Path(tempfile.gettempdir()) / f"gradata-{uid}" | ||
| user_tmp.mkdir(parents=True, exist_ok=True) | ||
| dir_hash = hashlib.md5(str(brain_dir).encode()).hexdigest()[:8] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Python hashlib md5 usedforsecurity parameter FIPS
💡 Result:
In Python’s hashlib, the usedforsecurity keyword-only parameter (added in Python 3.9) exists mainly to deal with FIPS-restricted environments where some digests (notably MD5 and often SHA1) may be blocked/disabled. By default it is True; setting it to False tells hashlib that you’re using the hash for a non-security purpose (e.g., checksums, internal IDs), and may allow the call to work even when FIPS rules would otherwise prohibit it. [1]
Typical usage:
import hashlib
h = hashlib.md5(b"data", usedforsecurity=False)Notes:
- On a normal (non-FIPS) Python/OpenSSL build,
usedforsecurityusually has no practical effect. [1] - On FIPS-enabled builds/distros, it can act as an escape hatch to permit “insecure” hashes in OpenSSL FIPS mode—but you remain responsible for compliance requirements. [2]
Sources: Python hashlib docs [1], RHEL 8.1 release notes on Python/OpenSSL FIPS + usedforsecurity [2].
[1] (docs.python.org)
[2] (access.redhat.com)
Citations:
- 1: https://docs.python.org/3/library/hashlib.html
- 2: https://access.redhat.com/documentation/en-en/red_hat_enterprise_linux/8/pdf/8.1_release_notes/red_hat_enterprise_linux-8-8.1_release_notes-en-us.pdf
🏁 Script executed:
# First, verify the file exists and check its content around line 68
if [ -f "src/gradata/hooks/pre_compact.py" ]; then
echo "=== File found ==="
wc -l src/gradata/hooks/pre_compact.py
echo ""
echo "=== Lines 60-75 (context around line 68) ==="
sed -n '60,75p' src/gradata/hooks/pre_compact.py
echo ""
echo "=== All hashlib.md5 usages ==="
rg "hashlib\.md5" src/gradata/hooks/pre_compact.py -n
else
echo "File not found at src/gradata/hooks/pre_compact.py"
fiRepository: Gradata/gradata
Length of output: 992
Add usedforsecurity=False to hashlib.md5() for FIPS compatibility.
On Python 3.9+ systems with FIPS enabled, hashlib.md5() without usedforsecurity=False raises ValueError. Since this hash is only for filename uniqueness, not cryptographic security, marking it as such ensures compatibility.
🔧 Proposed fix
- dir_hash = hashlib.md5(str(brain_dir).encode()).hexdigest()[:8]
+ dir_hash = hashlib.md5(str(brain_dir).encode(), usedforsecurity=False).hexdigest()[:8]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dir_hash = hashlib.md5(str(brain_dir).encode()).hexdigest()[:8] | |
| dir_hash = hashlib.md5(str(brain_dir).encode(), usedforsecurity=False).hexdigest()[:8] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/hooks/pre_compact.py` at line 68, The MD5 call used to generate
dir_hash is incompatible with FIPS-enabled Python because it omits the
usedforsecurity flag; update the call that constructs dir_hash in pre_compact.py
(the hashlib.md5(...) that encodes str(brain_dir)) to pass usedforsecurity=False
so the non-cryptographic filename-uniqueness hash works on FIPS systems, keeping
the rest of the logic (encoding and hexdigest slicing) unchanged.
| def main(data: dict) -> dict | None: | ||
| if parse_lessons is None: | ||
| return None | ||
|
|
||
| brain_dir = resolve_brain_dir() | ||
| if not brain_dir: | ||
| return None | ||
|
|
||
| lessons_path = Path(brain_dir) / "lessons.md" | ||
| if not lessons_path.is_file(): | ||
| return None | ||
|
|
||
| text = lessons_path.read_text(encoding="utf-8") | ||
| all_lessons = parse_lessons(text) | ||
| rule_lessons = [lesson for lesson in all_lessons if lesson.state.name == "RULE"] | ||
|
|
||
| if not rule_lessons: | ||
| return None | ||
|
|
||
| rules = [] | ||
| for lesson in rule_lessons[:MAX_REMINDERS]: | ||
| desc = lesson.description | ||
| truncated = desc[:120] + "..." if len(desc) > 120 else desc | ||
| rules.append(f"[RULE:{lesson.confidence:.2f}] {lesson.category}: {truncated}") | ||
|
|
||
| block = "ACTIVE RULES (learned from corrections):\n" + "\n".join(f" • {r}" for r in rules) | ||
| return {"result": block} |
There was a problem hiding this comment.
Add exception handling to prevent hook failures from blocking tool execution.
Per hook guidelines, errors must be silent. File I/O (read_text) and parse_lessons can raise exceptions that would propagate and potentially block the tool chain.
🛡️ Proposed fix
def main(data: dict) -> dict | None:
+ try:
- if parse_lessons is None:
- return None
+ if parse_lessons is None:
+ return None
- brain_dir = resolve_brain_dir()
- if not brain_dir:
- return None
+ brain_dir = resolve_brain_dir()
+ if not brain_dir:
+ return None
- lessons_path = Path(brain_dir) / "lessons.md"
- if not lessons_path.is_file():
- return None
+ lessons_path = Path(brain_dir) / "lessons.md"
+ if not lessons_path.is_file():
+ return None
- text = lessons_path.read_text(encoding="utf-8")
- all_lessons = parse_lessons(text)
- rule_lessons = [lesson for lesson in all_lessons if lesson.state.name == "RULE"]
+ text = lessons_path.read_text(encoding="utf-8")
+ all_lessons = parse_lessons(text)
+ rule_lessons = [lesson for lesson in all_lessons if lesson.state.name == "RULE"]
- if not rule_lessons:
- return None
+ if not rule_lessons:
+ return None
- rules = []
- for lesson in rule_lessons[:MAX_REMINDERS]:
- desc = lesson.description
- truncated = desc[:120] + "..." if len(desc) > 120 else desc
- rules.append(f"[RULE:{lesson.confidence:.2f}] {lesson.category}: {truncated}")
+ rules = []
+ for lesson in rule_lessons[:MAX_REMINDERS]:
+ desc = lesson.description
+ truncated = desc[:120] + "..." if len(desc) > 120 else desc
+ rules.append(f"[RULE:{lesson.confidence:.2f}] {lesson.category}: {truncated}")
- block = "ACTIVE RULES (learned from corrections):\n" + "\n".join(f" • {r}" for r in rules)
- return {"result": block}
+ block = "ACTIVE RULES (learned from corrections):\n" + "\n".join(f" • {r}" for r in rules)
+ return {"result": block}
+ except Exception:
+ pass
+ return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/hooks/rule_enforcement.py` around lines 24 - 50, Wrap the file
I/O and parsing steps in main with exception handling so no exceptions escape
the hook: around resolve_brain_dir(), lessons_path.read_text(...), and the
parse_lessons(text) call (and the loop that builds rules using MAX_REMINDERS),
catch broad exceptions, optionally log at debug if a logger exists, and return
None on error; ensure parse_lessons being None is still handled as before and
that the function never lets file or parsing errors propagate.
| def main(data: dict) -> dict | None: | ||
| tool_input = data.get("tool_input", {}) | ||
|
|
||
| # Collect all content to scan | ||
| contents_to_scan = [] | ||
| content = tool_input.get("content", "") or tool_input.get("new_string", "") | ||
| if content: | ||
| contents_to_scan.append(content) | ||
|
|
||
| # MultiEdit support: scan each edit's new_string | ||
| for edit in tool_input.get("edits", []): | ||
| edit_content = edit.get("new_string", "") | ||
| if edit_content: | ||
| contents_to_scan.append(edit_content) | ||
|
|
||
| if not contents_to_scan: | ||
| return None | ||
|
|
||
| findings = [] | ||
| for text in contents_to_scan: | ||
| findings.extend(_scan_content(text)) | ||
|
|
||
| if findings: | ||
| file_path = tool_input.get("file_path", "unknown") | ||
| names = ", ".join(f["name"] for f in findings) | ||
| return { | ||
| "decision": "block", | ||
| "reason": f"SECRET DETECTED: {len(findings)} potential secret(s) in {file_path}: {names}. Move secrets to .env or environment variables.", | ||
| } | ||
| return None |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider wrapping main in try/except for defensive error handling.
While run_hook catches exceptions at the outer level, other hooks in this PR (e.g., implicit_feedback.py, context_inject.py) wrap their main body in try/except returning None. This provides consistent defensive coding across hooks.
♻️ Optional enhancement for consistency
def main(data: dict) -> dict | None:
+ try:
tool_input = data.get("tool_input", {})
...
return None
+ except Exception:
+ return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def main(data: dict) -> dict | None: | |
| tool_input = data.get("tool_input", {}) | |
| # Collect all content to scan | |
| contents_to_scan = [] | |
| content = tool_input.get("content", "") or tool_input.get("new_string", "") | |
| if content: | |
| contents_to_scan.append(content) | |
| # MultiEdit support: scan each edit's new_string | |
| for edit in tool_input.get("edits", []): | |
| edit_content = edit.get("new_string", "") | |
| if edit_content: | |
| contents_to_scan.append(edit_content) | |
| if not contents_to_scan: | |
| return None | |
| findings = [] | |
| for text in contents_to_scan: | |
| findings.extend(_scan_content(text)) | |
| if findings: | |
| file_path = tool_input.get("file_path", "unknown") | |
| names = ", ".join(f["name"] for f in findings) | |
| return { | |
| "decision": "block", | |
| "reason": f"SECRET DETECTED: {len(findings)} potential secret(s) in {file_path}: {names}. Move secrets to .env or environment variables.", | |
| } | |
| return None | |
| def main(data: dict) -> dict | None: | |
| try: | |
| tool_input = data.get("tool_input", {}) | |
| # Collect all content to scan | |
| contents_to_scan = [] | |
| content = tool_input.get("content", "") or tool_input.get("new_string", "") | |
| if content: | |
| contents_to_scan.append(content) | |
| # MultiEdit support: scan each edit's new_string | |
| for edit in tool_input.get("edits", []): | |
| edit_content = edit.get("new_string", "") | |
| if edit_content: | |
| contents_to_scan.append(edit_content) | |
| if not contents_to_scan: | |
| return None | |
| findings = [] | |
| for text in contents_to_scan: | |
| findings.extend(_scan_content(text)) | |
| if findings: | |
| file_path = tool_input.get("file_path", "unknown") | |
| names = ", ".join(f["name"] for f in findings) | |
| return { | |
| "decision": "block", | |
| "reason": f"SECRET DETECTED: {len(findings)} potential secret(s) in {file_path}: {names}. Move secrets to .env or environment variables.", | |
| } | |
| return None | |
| except Exception: | |
| return None |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/hooks/secret_scan.py` around lines 44 - 73, The main function
lacks defensive error handling; wrap the body of main (which builds
contents_to_scan, iterates edits, calls _scan_content, and assembles the
findings/return value) in a try/except that catches Exception and returns None
on error to match other hooks (e.g., implicit_feedback.py, context_inject.py).
Ensure the try block includes all logic that reads tool_input, constructs
contents_to_scan, calls _scan_content, and builds the final decision payload,
and that the except logs or swallows the exception and returns None so failures
in _scan_content or unexpected input don't crash the hook.
| def main(data: dict) -> dict | None: | ||
| try: | ||
| brain_dir_str = resolve_brain_dir() | ||
| if not brain_dir_str: | ||
| return None | ||
| brain_dir = Path(brain_dir_str) | ||
|
|
||
| persist_dir = brain_dir / "sessions" / "persist" | ||
| persist_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| session = _get_session_number(brain_dir) | ||
| modified = _get_modified_files() | ||
|
|
||
| handoff = { | ||
| "timestamp": datetime.now(UTC).isoformat(), | ||
| "session": session, | ||
| "modified_files": modified[:50], | ||
| "file_count": len(modified), | ||
| } | ||
|
|
||
| filename = f"session-{session}.json" if session else "session-unknown.json" | ||
| out_path = persist_dir / filename | ||
| out_path.write_text(json.dumps(handoff, indent=2), encoding="utf-8") | ||
| except Exception: | ||
| pass | ||
| return None |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant exception handling.
The run_hook() function already wraps main_fn execution in a try/except block (per _base.py:96-97). The try/except here is defensive but redundant.
♻️ Optional simplification
def main(data: dict) -> dict | None:
- try:
- brain_dir_str = resolve_brain_dir()
- if not brain_dir_str:
- return None
- brain_dir = Path(brain_dir_str)
+ brain_dir_str = resolve_brain_dir()
+ if not brain_dir_str:
+ return None
+ brain_dir = Path(brain_dir_str)
- persist_dir = brain_dir / "sessions" / "persist"
- persist_dir.mkdir(parents=True, exist_ok=True)
+ persist_dir = brain_dir / "sessions" / "persist"
+ persist_dir.mkdir(parents=True, exist_ok=True)
- session = _get_session_number(brain_dir)
- modified = _get_modified_files()
+ session = _get_session_number(brain_dir)
+ modified = _get_modified_files()
- handoff = {
- "timestamp": datetime.now(UTC).isoformat(),
- "session": session,
- "modified_files": modified[:50],
- "file_count": len(modified),
- }
+ handoff = {
+ "timestamp": datetime.now(UTC).isoformat(),
+ "session": session,
+ "modified_files": modified[:50],
+ "file_count": len(modified),
+ }
- filename = f"session-{session}.json" if session else "session-unknown.json"
- out_path = persist_dir / filename
- out_path.write_text(json.dumps(handoff, indent=2), encoding="utf-8")
- except Exception:
- pass
+ filename = f"session-{session}.json" if session else "session-unknown.json"
+ out_path = persist_dir / filename
+ out_path.write_text(json.dumps(handoff, indent=2), encoding="utf-8")
return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/hooks/session_persist.py` around lines 68 - 93, Remove the
redundant try/except in the main function of session_persist.py: delete the
surrounding try/except block so main(data: dict) executes its logic directly and
lets run_hook's wrapper (run_hook in _base.py) handle exceptions; ensure the
function still returns None at the end and keeps all existing behavior
(resolve_brain_dir(), _get_session_number(), _get_modified_files(), file write
to persist_dir) unchanged.
| client.chat.completions.create = patched_create | ||
| client._brain = brain # Expose for correction tracking |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check Brain class for thread-safety indicators and mutable state.
# Look for threading primitives in Brain class
rg -B 3 -A 10 'class Brain' --type py -g '*brain.py' | rg -i 'lock|thread|mutex|semaphore|concurrent'
# Check for mutable instance attributes that could cause race conditions
ast-grep --pattern 'class Brain:
$$$
def __init__($$$):
$$$
self.$ATTR = $$$
$$$'
# Look for methods that modify state
rg 'def (apply_brain_rules|log_output|observe)\(' --type py -A 15 -g '*brain.py'Repository: Gradata/gradata
Length of output: 3709
🏁 Script executed:
# Get full Brain class definition to understand state management
cat -n src/gradata/brain.py | head -100Repository: Gradata/gradata
Length of output: 4393
🏁 Script executed:
# Get the full context around lines 98-99 in openai_adapter.py
cat -n src/gradata/integrations/openai_adapter.py | sed -n '80,120p'Repository: Gradata/gradata
Length of output: 1001
🏁 Script executed:
# Check QueryBudget and Cloud classes for thread-safety
fd -e py -x grep -l 'class QueryBudget\|class Cloud' {} \;Repository: Gradata/gradata
Length of output: 127
🏁 Script executed:
# Look for concurrent/threading usage patterns in the codebase
rg 'Thread|Lock|RLock|Semaphore|concurrent|asyncio' --type py -c | head -20Repository: Gradata/gradata
Length of output: 814
🏁 Script executed:
# Get full context of patched_create function definition
cat -n src/gradata/integrations/openai_adapter.py | sed -n '50,100p'Repository: Gradata/gradata
Length of output: 2374
🏁 Script executed:
# Check if there's any documentation or guards about thread safety in openai_adapter
cat -n src/gradata/integrations/openai_adapter.py | head -60Repository: Gradata/gradata
Length of output: 2301
🏁 Script executed:
# Check QueryBudget class for mutable state and thread safety
cat -n src/gradata/security/query_budget.py | head -80Repository: Gradata/gradata
Length of output: 3342
🏁 Script executed:
# Check if there are async variants and how they handle the brain
rg 'async def.*create|AsyncOpenAI' --type py src/gradata/integrations/Repository: Gradata/gradata
Length of output: 167
Document that patched client must not be shared across threads, or add thread-safety mechanism for Brain instance.
The Brain class is explicitly documented as NOT thread-safe due to SQLite limitations (see Brain docstring: "Brain is NOT thread-safe... Use one Brain per process, or use process-level locks"). Storing the Brain instance on the client at line 99 creates a race condition when the client is shared across threads. Every client.chat.completions.create() call will invoke brain.log_output() and brain.observe() (lines 89, 92), which perform SQLite writes. Concurrent calls from multiple threads will trigger SQLite's concurrent write constraints and cause data corruption or runtime errors.
Either: (1) add thread safety documentation that the patched client must not be shared across threads, or (2) implement a locking mechanism (e.g., threading.RLock) around Brain state access in the patched_create function.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/integrations/openai_adapter.py` around lines 98 - 99, The patched
client currently stores Brain on client._brain and calls brain.log_output() and
brain.observe() inside patched_create, which is unsafe for multi-threaded use;
fix by adding a threading.RLock (e.g., client._brain_lock = threading.RLock())
when you attach the brain and wrap all Brain accesses in patched_create (acquire
the lock around calls to brain.log_output() and brain.observe()) to serialize
SQLite writes; alternatively, if you prefer not to add locking, update the
module/class docstring and add a clear comment where client._brain is set
stating the patched client MUST NOT be shared across threads and recommend one
Brain per process or using process-level locks (reference symbols: Brain,
patched_create, client._brain, client.chat.completions.create,
brain.log_output(), brain.observe()).
sdk-ci.yml was referencing working-directory: sdk/ which no longer exists. Updated to match PR #20's version (root-level src/gradata/). Added bandit skip rules for false positives. Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/sdk-ci.yml (1)
36-40:⚠️ Potential issue | 🟡 MinorPin
uvinstead of usinglatestLine 39 keeps CI non-deterministic (
version: "latest"). Pin to a stable release line (e.g.,0.11.x) for reproducible runs.♻️ Suggested fix
- name: Install uv uses: astral-sh/setup-uv@v4 with: - version: "latest" + version: "0.11.x"What version pinning strategy is currently recommended for astral-sh/setup-uv in GitHub Actions to ensure reproducible CI runs?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sdk-ci.yml around lines 36 - 40, The CI step named "Install uv" currently uses version: "latest" which makes runs non-deterministic; update that step (the GitHub Actions step using astral-sh/setup-uv@v4, identified by the step name "Install uv" and the version key) to pin the UV runtime to a stable release line (for example change version: "latest" to version: "0.11.x" or a specific patch release) so workflow executions become reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Around line 78-87: The global Ruff ignore list currently contains broad
exemptions for codes "E402", "N806", "N802", "N814", "RUF001", "RUF002",
"RUF003", and "TC003" which weakens repo-wide linting; remove these codes from
the global ignore/extend-ignore block in pyproject.toml and instead apply
targeted exceptions: add per-file-ignores entries for specific files that truly
need them or use inline noqa/# ruff: noqa comments on the exact lines that
justify the exception (e.g., imports that must be conditional for "E402" or
intentional naming for "N806"/"N802"/"N814"), and keep the global ignore minimal
so only universally-accepted rules are disabled.
- Around line 105-118: The pyproject.toml currently disables many Bandit checks
globally; remove the repo-wide skips and instead add scoped inline suppressions
(using "# nosec <rule>") at the specific callsites that need them: mark
subprocess calls in enhancements/git_backfill.py and hooks/session_persist.py
for B603/B607, mark SQL-building functions that use parameterized inputs for
B608 where queries are safely parameterized, and mark validated URL opens in
daemon.py and enhancements/llm_synthesizer.py for B310; start by removing B603,
B607, B608, B310 from the skips list and add explicit "# nosec
B603/B607/B608/B310" comments immediately next to the offending
subprocess/sql/urlopen calls (or wrapper functions) so Bandit still scans the
rest of the repo.
---
Duplicate comments:
In @.github/workflows/sdk-ci.yml:
- Around line 36-40: The CI step named "Install uv" currently uses version:
"latest" which makes runs non-deterministic; update that step (the GitHub
Actions step using astral-sh/setup-uv@v4, identified by the step name "Install
uv" and the version key) to pin the UV runtime to a stable release line (for
example change version: "latest" to version: "0.11.x" or a specific patch
release) so workflow executions become reproducible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7b0ab9b-29ce-40e5-be64-409804eb13af
📒 Files selected for processing (2)
.github/workflows/sdk-ci.ymlpyproject.toml
📜 Review details
🔇 Additional comments (1)
.github/workflows/sdk-ci.yml (1)
53-57: Good separation of main tests vs spec-compliance gateRunning
tests/first andtests/test_spec_compliance.pyseparately is a solid CI split for optional/cloud-dependent imports while still preserving a dedicated spec check.
| "E741", # ambiguous variable name — used intentionally in list comprehensions | ||
| "E402", # module-level import not at top — SDK uses conditional/lazy imports | ||
| "N806", # non-lowercase variable in function — intentional for constants | ||
| "N802", # invalid function name — intentional aliasing | ||
| "N814", # camelcase imported as constant — intentional aliasing | ||
| "RUF001", # ambiguous unicode in string — intentional en-dash in regex | ||
| "RUF002", # ambiguous unicode in docstring — intentional | ||
| "RUF003", # ambiguous unicode in comment — intentional | ||
| "TC003", # move stdlib import to TYPE_CHECKING — false positive with runtime Path usage | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Scope Ruff ignores to files instead of global ignore list
Lines 78-87 add many global ignores (E402, naming, RUF00x, TC003). That weakens linting across the entire repo. Prefer per-file ignores/inline noqa for justified exceptions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` around lines 78 - 87, The global Ruff ignore list currently
contains broad exemptions for codes "E402", "N806", "N802", "N814", "RUF001",
"RUF002", "RUF003", and "TC003" which weakens repo-wide linting; remove these
codes from the global ignore/extend-ignore block in pyproject.toml and instead
apply targeted exceptions: add per-file-ignores entries for specific files that
truly need them or use inline noqa/# ruff: noqa comments on the exact lines that
justify the exception (e.g., imports that must be conditional for "E402" or
intentional naming for "N806"/"N802"/"N814"), and keep the global ignore minimal
so only universally-accepted rules are disabled.
| skips = [ | ||
| "B101", # allow assert in non-test code | ||
| "B110", # try/except/pass — SDK uses defensive coding with logged fallbacks | ||
| "B112", # try/except/continue — same pattern | ||
| "B311", # pseudo-random — used for non-security shuffling (rule ordering) | ||
| "B324", # weak hash — SHA256 used for content hashing, not security | ||
| "B404", # subprocess import — SDK shells out to qmd/git intentionally | ||
| "B603", # subprocess without shell — intentional CLI tool invocations | ||
| "B607", # partial path — CLI tools resolved via PATH | ||
| "B608", # SQL string construction — SDK builds SQLite queries with parameterized inputs | ||
| "B310", # url open audit — SDK opens URLs from validated config, not user input | ||
| "B105", # hardcoded password — false positive on string constants like 'pass', 'False' | ||
| "B615", # huggingface download — benchmark code, not production | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: show global skips and potentially affected API usage.
python - <<'PY'
import tomllib, pathlib
cfg = tomllib.loads(pathlib.Path("pyproject.toml").read_text())
print("Bandit skips:", cfg["tool"]["bandit"]["skips"])
PY
rg -n -C2 'subprocess\.(run|Popen)\(|urllib\.request\.urlopen\(|execute\(' src/gradataRepository: Gradata/gradata
Length of output: 50497
Replace global Bandit skips with scoped suppressions to maintain security scanner signal
Lines 105–118 suppress 12 high-signal security rules repo-wide, reducing Bandit's ability to detect new vulnerabilities in unrelated code. While each skip has a legitimate local justification (parameterized SQL, trusted subprocess invocations, validated config URLs), the global approach sacrifices future coverage.
Move these to inline # nosec <rule> comments at specific callsites instead. This retains the suppressions where needed while allowing Bandit to flag future regressions elsewhere. Given the widespread patterns across the codebase (subprocess in enhancements/git_backfill.py, hooks/session_persist.py; SQL in multiple modules; urlopen in daemon.py, enhancements/llm_synthesizer.py), scoped suppressions require effort but significantly improve security posture.
Start with the highest-risk rules (B603, B607, B608, B310) and incrementally annotate callsites rather than suppressing repo-wide.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` around lines 105 - 118, The pyproject.toml currently disables
many Bandit checks globally; remove the repo-wide skips and instead add scoped
inline suppressions (using "# nosec <rule>") at the specific callsites that need
them: mark subprocess calls in enhancements/git_backfill.py and
hooks/session_persist.py for B603/B607, mark SQL-building functions that use
parameterized inputs for B608 where queries are safely parameterized, and mark
validated URL opens in daemon.py and enhancements/llm_synthesizer.py for B310;
start by removing B603, B607, B608, B310 from the skips list and add explicit "#
nosec B603/B607/B608/B310" comments immediately next to the offending
subprocess/sql/urlopen calls (or wrapper functions) so Bandit still scans the
rest of the repo.
Same fix as PR #20 — 12 corrections across 3 sessions instead of 8 in one. Co-Authored-By: Gradata <noreply@gradata.ai>
- Re-sign rules after patching to prevent HMAC verification failures - Restrict patch_rule() to brain-local lessons.md only - Preserve semantic word ordering in deterministic patches - Normalize whitespace in rule description comparison - Use json_extract() instead of LIKE for precise JSON matching - Use context manager for SQLite connections in rule_integrity - Extract event whitelist to module-level constant in hooks/_base - Clarify one-per-category signature design in docstring Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
src/gradata/enhancements/rule_canary.py (3)
80-90:⚠️ Potential issue | 🟡 MinorRemove the hardcoded five-parent DB fallback.
The
parent.parent.parent.parent.parent / "brain"probe is still layout-dependent and will fail as soon as the package is installed somewhere else or the source tree moves. That leaves canary state resolution silently broken wheneverctx/BRAIN_DIRare absent.As per coding guidelines,
src/gradata/**/*.py: "no hardcoded paths".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/rule_canary.py` around lines 80 - 90, The fallback that uses Path(__file__).resolve().parent.parent.parent.parent.parent / "brain" is a hardcoded layout and must be removed; replace it with a layout-agnostic probe that either (a) walks up Path(__file__).resolve().parents and returns the first parent / "brain" that exists (limit depth to e.g. 10), or (b) falls back to locating the packaged resource via importlib.resources or pkgutil if the module is installed, then check for "system.db" there; update the logic around scripts_dir and p in rule_canary.py to perform this safe parent iteration (or resource lookup) and remove the fixed five-parent expression so resolution no longer depends on a specific source layout.
150-154:⚠️ Potential issue | 🟠 MajorCount corrections from
$.category, not$.rule_category.
brain.correct()writes correction category underdata_json.category, so this query never matches normalCORRECTIONrows. The canary will therefore see zero regressions and can promote rules that should have been rolled back.🐛 Minimal fix
corr_row = conn.execute( "SELECT COUNT(*) as cnt FROM events WHERE type = 'CORRECTION' " - "AND json_extract(data_json, '$.rule_category') = ? " + "AND json_extract(data_json, '$.category') = ? " "AND CAST(session AS INTEGER) >= ?", (rule_category, start_session), ).fetchone()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/rule_canary.py` around lines 150 - 154, The SQL counting corrections is using the wrong JSON key (json_extract(data_json, '$.rule_category')) so CORRECTION rows written by brain.correct() (which use data_json.category) never match; update the query in rule_canary.py (the conn.execute call that assigns corr_row) to use json_extract(data_json, '$.category') instead of '$.rule_category' so the COUNT(*) correctly counts corrections for the given rule_category and start_session.
215-227:⚠️ Potential issue | 🟠 MajorPass the current
BrainContextinto both canary lifecycle emits.These emits still rely on global path resolution even though the status change was applied against a specific
db_path. In multi-brain processes that can writeRULE_ROLLBACK/CANARY_PROMOTEDinto the wrong brain's event log.🔧 Sketch of the fix
try: from gradata._events import emit + from gradata._paths import BrainContext + ctx = BrainContext.from_brain_dir(db_path.parent) emit( "RULE_ROLLBACK", "rule_canary:rollback_rule", { "rule_category": rule_category, @@ }, tags=[f"category:{rule_category}", "canary:rollback"], + ctx=ctx, )try: from gradata._events import emit + from gradata._paths import BrainContext + ctx = BrainContext.from_brain_dir(db_path.parent) emit( "CANARY_PROMOTED", "rule_canary:promote_to_active", {"rule_category": rule_category}, tags=[f"category:{rule_category}", "canary:promoted"], + ctx=ctx, )As per coding guidelines,
src/gradata/**/*.py: "all functions accepting BrainContext where DB access occurs".Also applies to: 252-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/rule_canary.py` around lines 215 - 227, The RULE_ROLLBACK and CANARY_PROMOTED emits in rule_canary.py must be called with the current BrainContext so events are written to the correct brain; update both emit(...) calls (the one emitting "RULE_ROLLBACK" and the one emitting "CANARY_PROMOTED") to pass the function's BrainContext parameter (e.g., the local variable holding the BrainContext such as ctx or brain_context) into emit as the context argument (use the emit signature's context kwarg or the appropriate positional slot) so the event logging uses the specific db_path rather than global resolution.src/gradata/enhancements/rule_integrity.py (1)
217-246:⚠️ Potential issue | 🟠 MajorSignature storage is still lossy for duplicate categories.
rule_signatures.categoryremains the primary key, andstore_signature()replaces oncategory.upper(). This repo already carries multiple same-category rules at once (for examplePROCESSappears several times intests/test_pattern_graduation_integration.py), so only one of them can have a valid stored signature. Any verification keyed only by category will report false tampering for the others.Also applies to: 264-276
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/rule_integrity.py` around lines 217 - 246, The rule_signatures table uses category as PRIMARY KEY so store_signature (which inserts category.upper()) overwrites multiple rules in the same category; change the schema and storage key to be per-rule (e.g., add a rule_id or rule_name column and make PRIMARY KEY either (rule_id) or a composite (category, rule_name)); update the table creation in rule_signatures and modify store_signature to accept and persist the unique rule identifier instead of only category.upper(); also update the corresponding verification code (the related function that reads signatures—the other block referenced around lines 264-276) to look up by the new unique key rather than by category.src/gradata/brain.py (1)
357-372:⚠️ Potential issue | 🟠 Major
patch_rule()still half-applies on post-write failure.Line 357 rewrites
lessons.mdbefore any visible audit-row persistence, and the signature update plusRULE_PATCHEDemit happen afterward. If either of those steps fails, the rule text is already mutated on disk but the integrity state/audit trail is missing, which breaks the PR contract forrule_patchesand leaves the patch only partially applied.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/brain.py` around lines 357 - 372, The patch currently writes lessons.md via write_lessons_safe before completing audit/signature persistence and emitting, which can leave a partial state if later steps fail; reorder and make the update atomic by first completing all persistence/signing/audit steps (call sign_and_store and persist the rule-patch audit row and call self.emit("RULE_PATCHED", ...)) and only after those succeed perform the final filesystem update (write_lessons_safe), or alternatively write lessons to a temp file and atomically rename it after sign_and_store and emit succeed; adjust patch_rule to use write-lessons-after-persist (or temp+rename) and reference write_lessons_safe, sign_and_store and self.emit within the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gradata/enhancements/self_healing.py`:
- Around line 172-185: The patch generator is nondeterministic because
new_context_words is a set and iteration order is hash-randomized; change the
logic to derive context words in a deterministic order (e.g., preserve the
original order from correction_lower.split()) by iterating over
correction_lower.split(), selecting tokens not in rule_lower.split() and not in
the stop-word set, collecting up to three tokens, and then build context_phrase
from that ordered list; update the code that computes
new_context_words/context_phrase (referencing new_context_words, rule_lower,
correction_lower, and rule_description) so the same input always yields the same
output.
In `@src/gradata/hooks/_base.py`:
- Around line 37-43: read_input currently returns whatever json.loads produces
so non-object JSON (list/str/number) can slip through; change read_input (and
its use in run_hook) to validate that json.loads(raw) yields a dict (mapping)
and return None for any other JSON type or parsing error. Specifically, in
read_input ensure the parsed value is an instance of dict (or typing.Mapping)
before returning it, otherwise return None so downstream code in run_hook that
expects data.get(...) never receives non-object values.
---
Duplicate comments:
In `@src/gradata/brain.py`:
- Around line 357-372: The patch currently writes lessons.md via
write_lessons_safe before completing audit/signature persistence and emitting,
which can leave a partial state if later steps fail; reorder and make the update
atomic by first completing all persistence/signing/audit steps (call
sign_and_store and persist the rule-patch audit row and call
self.emit("RULE_PATCHED", ...)) and only after those succeed perform the final
filesystem update (write_lessons_safe), or alternatively write lessons to a temp
file and atomically rename it after sign_and_store and emit succeed; adjust
patch_rule to use write-lessons-after-persist (or temp+rename) and reference
write_lessons_safe, sign_and_store and self.emit within the change.
In `@src/gradata/enhancements/rule_canary.py`:
- Around line 80-90: The fallback that uses
Path(__file__).resolve().parent.parent.parent.parent.parent / "brain" is a
hardcoded layout and must be removed; replace it with a layout-agnostic probe
that either (a) walks up Path(__file__).resolve().parents and returns the first
parent / "brain" that exists (limit depth to e.g. 10), or (b) falls back to
locating the packaged resource via importlib.resources or pkgutil if the module
is installed, then check for "system.db" there; update the logic around
scripts_dir and p in rule_canary.py to perform this safe parent iteration (or
resource lookup) and remove the fixed five-parent expression so resolution no
longer depends on a specific source layout.
- Around line 150-154: The SQL counting corrections is using the wrong JSON key
(json_extract(data_json, '$.rule_category')) so CORRECTION rows written by
brain.correct() (which use data_json.category) never match; update the query in
rule_canary.py (the conn.execute call that assigns corr_row) to use
json_extract(data_json, '$.category') instead of '$.rule_category' so the
COUNT(*) correctly counts corrections for the given rule_category and
start_session.
- Around line 215-227: The RULE_ROLLBACK and CANARY_PROMOTED emits in
rule_canary.py must be called with the current BrainContext so events are
written to the correct brain; update both emit(...) calls (the one emitting
"RULE_ROLLBACK" and the one emitting "CANARY_PROMOTED") to pass the function's
BrainContext parameter (e.g., the local variable holding the BrainContext such
as ctx or brain_context) into emit as the context argument (use the emit
signature's context kwarg or the appropriate positional slot) so the event
logging uses the specific db_path rather than global resolution.
In `@src/gradata/enhancements/rule_integrity.py`:
- Around line 217-246: The rule_signatures table uses category as PRIMARY KEY so
store_signature (which inserts category.upper()) overwrites multiple rules in
the same category; change the schema and storage key to be per-rule (e.g., add a
rule_id or rule_name column and make PRIMARY KEY either (rule_id) or a composite
(category, rule_name)); update the table creation in rule_signatures and modify
store_signature to accept and persist the unique rule identifier instead of only
category.upper(); also update the corresponding verification code (the related
function that reads signatures—the other block referenced around lines 264-276)
to look up by the new unique key rather than by category.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c6d684d7-62f5-46b3-941d-bd66dd157cd6
📒 Files selected for processing (6)
src/gradata/brain.pysrc/gradata/enhancements/rule_canary.pysrc/gradata/enhancements/rule_integrity.pysrc/gradata/enhancements/self_healing.pysrc/gradata/hooks/_base.pytests/test_pattern_graduation_integration.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
tests/**
⚙️ CodeRabbit configuration file
tests/**: Test files. Verify: no hardcoded paths, assertions check specific values not just truthiness,
parametrized tests preferred for boundary conditions, floating point comparisons use pytest.approx.
Files:
tests/test_pattern_graduation_integration.py
src/gradata/**/*.py
⚙️ CodeRabbit configuration file
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required), no print()
statements (use logging), all functions accepting BrainContext where DB access occurs, no hardcoded paths. Severity
scoring must clamp to [0,1]. Confidence values must be in [0.0, 1.0].
Files:
src/gradata/brain.pysrc/gradata/hooks/_base.pysrc/gradata/enhancements/rule_canary.pysrc/gradata/enhancements/rule_integrity.pysrc/gradata/enhancements/self_healing.py
src/gradata/hooks/**
⚙️ CodeRabbit configuration file
src/gradata/hooks/**: JavaScript hooks for Claude Code integration. Check for: no shell injection (no execSync with user
input), temp files must use per-user subdirectory, HTTP calls must have timeouts, errors must be silent (never block
the tool chain).
Files:
src/gradata/hooks/_base.py
| # Find words in correction that aren't in the rule -- these are the context | ||
| rule_words = set(rule_lower.split()) | ||
| correction_words = set(correction_lower.split()) | ||
| new_context_words = correction_words - rule_words - { | ||
| "the", "a", "an", "in", "on", "at", "to", "for", "of", "is", "was", | ||
| "from", "with", "and", "or", "but", "not", "this", "that", | ||
| } | ||
|
|
||
| if not new_context_words: | ||
| return rule_description # Can't narrow -- return unchanged | ||
|
|
||
| # Take top 3 most informative context words | ||
| context_phrase = " ".join(list(new_context_words)[:3]) | ||
| return f"{rule_description} (especially in context: {context_phrase})" |
There was a problem hiding this comment.
The “deterministic” patch generator is still hash-order dependent.
new_context_words is a set, and list(new_context_words)[:3] produces process-dependent ordering because Python randomizes hash iteration. That makes the generated patch text, retroactive gate result, and any downstream audit/event data non-reproducible.
🔁 Deterministic alternative
- new_context_words = correction_words - rule_words - {
+ new_context_words = correction_words - rule_words - {
"the", "a", "an", "in", "on", "at", "to", "for", "of", "is", "was",
"from", "with", "and", "or", "but", "not", "this", "that",
}
@@
- context_phrase = " ".join(list(new_context_words)[:3])
+ ordered_context_words = sorted(new_context_words)
+ context_phrase = " ".join(ordered_context_words[:3])
return f"{rule_description} (especially in context: {context_phrase})"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Find words in correction that aren't in the rule -- these are the context | |
| rule_words = set(rule_lower.split()) | |
| correction_words = set(correction_lower.split()) | |
| new_context_words = correction_words - rule_words - { | |
| "the", "a", "an", "in", "on", "at", "to", "for", "of", "is", "was", | |
| "from", "with", "and", "or", "but", "not", "this", "that", | |
| } | |
| if not new_context_words: | |
| return rule_description # Can't narrow -- return unchanged | |
| # Take top 3 most informative context words | |
| context_phrase = " ".join(list(new_context_words)[:3]) | |
| return f"{rule_description} (especially in context: {context_phrase})" | |
| # Find words in correction that aren't in the rule -- these are the context | |
| rule_words = set(rule_lower.split()) | |
| correction_words = set(correction_lower.split()) | |
| new_context_words = correction_words - rule_words - { | |
| "the", "a", "an", "in", "on", "at", "to", "for", "of", "is", "was", | |
| "from", "with", "and", "or", "but", "not", "this", "that", | |
| } | |
| if not new_context_words: | |
| return rule_description # Can't narrow -- return unchanged | |
| # Take top 3 most informative context words | |
| ordered_context_words = sorted(new_context_words) | |
| context_phrase = " ".join(ordered_context_words[:3]) | |
| return f"{rule_description} (especially in context: {context_phrase})" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/enhancements/self_healing.py` around lines 172 - 185, The patch
generator is nondeterministic because new_context_words is a set and iteration
order is hash-randomized; change the logic to derive context words in a
deterministic order (e.g., preserve the original order from
correction_lower.split()) by iterating over correction_lower.split(), selecting
tokens not in rule_lower.split() and not in the stop-word set, collecting up to
three tokens, and then build context_phrase from that ordered list; update the
code that computes new_context_words/context_phrase (referencing
new_context_words, rule_lower, correction_lower, and rule_description) so the
same input always yields the same output.
| def read_input(raw: str) -> dict | None: | ||
| if not raw or not raw.strip(): | ||
| return None | ||
| try: | ||
| return json.loads(raw) | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
Reject non-object JSON at the parser boundary.
read_input() is typed as dict | None, but json.loads() can return a list, string, or number. run_hook() then hands that value to hook code that expects data.get(...), so syntactically valid but non-object JSON becomes a suppressed runtime error and the hook silently does nothing.
🛠️ Minimal fix
def read_input(raw: str) -> dict | None:
if not raw or not raw.strip():
return None
try:
- return json.loads(raw)
+ data = json.loads(raw)
+ return data if isinstance(data, dict) else None
except Exception:
return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/hooks/_base.py` around lines 37 - 43, read_input currently
returns whatever json.loads produces so non-object JSON (list/str/number) can
slip through; change read_input (and its use in run_hook) to validate that
json.loads(raw) yields a dict (mapping) and return None for any other JSON type
or parsing error. Specifically, in read_input ensure the parsed value is an
instance of dict (or typing.Mapping) before returning it, otherwise return None
so downstream code in run_hook that expects data.get(...) never receives
non-object values.
Co-Authored-By: Gradata <noreply@gradata.ai>
Co-Authored-By: Gradata <noreply@gradata.ai>
…lway deploy (#22) * feat(cloud): scaffold FastAPI project with config Co-Authored-By: Gradata <noreply@gradata.ai> * feat(cloud): Supabase schema with RLS policies 8 tables (workspaces, workspace_members, brains, corrections, lessons, meta_rules, events, rule_patches), indexes, RLS policies for data isolation, and auto-create-workspace trigger on user signup. Co-Authored-By: Gradata <noreply@gradata.ai> * feat(cloud): Supabase client + API key/JWT auth with tests Async httpx wrapper for PostgREST (select/insert/upsert), API key lookup for SDK clients, HS256 JWT verification for dashboard users, and 5 passing tests with MockSupabaseClient fixture. Co-Authored-By: Gradata <noreply@gradata.ai> * feat(cloud): Pydantic request/response models for sync API SyncRequest, SyncResponse, CorrectionPayload, LessonPayload, EventPayload, MetaRulePayload with Severity/LessonState enums, confidence bounds validation, and 5 passing tests. Co-Authored-By: Gradata <noreply@gradata.ai> * feat(cloud): middleware, health endpoint, and route scaffolding CORS + request logging middleware, GET /health endpoint, route aggregation with stub sync/brains routers for Tasks 6-7. Co-Authored-By: Gradata <noreply@gradata.ai> * feat(cloud): sync + brains endpoints, Railway config, 17 tests green Implements POST /api/v1/sync (corrections, lessons, events, meta-rules), POST /api/v1/brains/connect, GET /api/v1/brains, and Railway deployment config. All 17 tests pass (auth: 5, models: 5, sync: 5, brains: 2). Co-Authored-By: Gradata <noreply@gradata.ai> * fix(cloud): address CodeRabbit + Greptile review feedback - Cache get_settings() with @lru_cache(maxsize=1) - Disable openapi_url and redoc_url in production - Add asynccontextmanager lifespan to clean up httpx client - Wrap log_requests middleware in try/except so errors get logged - Project out api_key column from brains list endpoint - Update last_sync_at after successful sync - Reorder .env.example vars alphabetically - Add update() method to SupabaseClient and mock Co-Authored-By: Gradata <noreply@gradata.ai> * fix(lint): resolve all ruff errors + add ruff to dev deps Same fixes as other branches: trailing newlines, import sorting, raise-from, collapsible-if, contextlib.suppress, pyright type ignore. Added ruff>=0.4 to dev deps and ignore rules for intentional patterns. Co-Authored-By: Gradata <noreply@gradata.ai> * fix(ci): update sdk-ci.yml paths + skip bandit false positives Same CI fixes as PR #20 and #21 branches. Co-Authored-By: Gradata <noreply@gradata.ai> * fix: address PR #22 CodeRabbit review findings - SQL: add UNIQUE(workspace_id, user_id) on brains table - SQL: drop duplicate idx_brains_api_key (UNIQUE already creates index) - SQL: pin search_path on SECURITY DEFINER trigger function - SQL: create default brain in handle_new_user trigger - brain.py: wrap patch_rule read-mutate-write under lessons_lock - brain.py: protect emit from making successful patch look failed - behavioral_extractor.py: align fallback _FACTUAL_RE with edit_classifier - behavioral_extractor.py: remove capitalization bypass in _is_actionable - conftest.py: mock select() now respects filters - sync.py: preserve created_at from SDK payload on corrections/events Co-Authored-By: Gradata <noreply@gradata.ai> * fix(lint): inline SIM103 condition in behavioral_extractor Co-Authored-By: Gradata <noreply@gradata.ai> * fix(test): stabilize graduation test for Python 3.12 Same fix as PR #20/#21 — 12 corrections across 3 sessions. Co-Authored-By: Gradata <noreply@gradata.ai> --------- Co-authored-by: Oliver Le <oliver@gradata.com> Co-authored-by: Gradata <noreply@gradata.ai>
| "AND json_extract(data_json, '$.rule_category') = ? " | ||
| "AND CAST(session AS INTEGER) >= ?", | ||
| (rule_category, start_session), |
There was a problem hiding this comment.
Wrong JSON field name silently disables rollback detection
The fix correctly moves away from LIKE, but uses the field name $.rule_category, which does not exist in CORRECTION event payloads. _core.py (line 166) stores the correction category under the key "category":
data = {
...
"category": category or "UNKNOWN",
...
}Because json_extract(data_json, '$.rule_category') will always return NULL for every CORRECTION event, correction_count will permanently be 0, and no canary rule will ever be recommended for ROLLBACK — even when a new rule is actively causing regressions. The canary's entire protective function is silently defeated.
| "AND json_extract(data_json, '$.rule_category') = ? " | |
| "AND CAST(session AS INTEGER) >= ?", | |
| (rule_category, start_session), | |
| "AND json_extract(data_json, '$.category') = ? " |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gradata/enhancements/rule_canary.py
Line: 152-154
Comment:
**Wrong JSON field name silently disables rollback detection**
The fix correctly moves away from `LIKE`, but uses the field name `$.rule_category`, which does not exist in `CORRECTION` event payloads. `_core.py` (line 166) stores the correction category under the key `"category"`:
```python
data = {
...
"category": category or "UNKNOWN",
...
}
```
Because `json_extract(data_json, '$.rule_category')` will always return `NULL` for every CORRECTION event, `correction_count` will permanently be 0, and no canary rule will ever be recommended for `ROLLBACK` — even when a new rule is actively causing regressions. The canary's entire protective function is silently defeated.
```suggestion
"AND json_extract(data_json, '$.category') = ? "
```
How can I resolve this? If you propose a fix, please make it concise.brain.auto_heal() reads recent RULE_FAILURE events, runs review_rule_failures, gates each candidate through retroactive_test, and applies the survivors via brain.patch_rule. Batches are deduped by (category, original_description) and hard-capped at max_patches (default 5) so a single session can't rewrite a rule multiple times. Before this commit, PR #21 emitted RULE_FAILURE but nothing consumed it. The full flow required the caller to manually call review_rule_failures + brain.patch_rule as the E2E test does. Adds auto_heal_failures(brain, ...) orchestrator in self_healing.py and Brain.auto_heal public method that delegates to it. 6 new tests covering empty input, passing candidate, retro-test failure, in-batch dedup, max_patches cap, and event-log read when no events are passed.
When brain.correct() detects a RULE_FAILURE it now invokes auto_heal_failures inline, capped at one patch per call so a single correction can rewrite at most the rule it just failed. Skipped in dry_run, approval_required, and renter modes; opt-out via correct(auto_heal=False). Adds auto_heal kwarg to brain.correct + brain_correct in _core.py and surfaces the heal summary on event["auto_healed"] when a patch lands. 3 new tests covering the default-on path, the opt-out path, and the dry_run skip. Closes the diagnose-then-do-nothing gap that PR #21 left open.
brain.auto_heal() reads recent RULE_FAILURE events, runs review_rule_failures, gates each candidate through retroactive_test, and applies the survivors via brain.patch_rule. Batches are deduped by (category, original_description) and hard-capped at max_patches (default 5) so a single session can't rewrite a rule multiple times. Before this commit, PR #21 emitted RULE_FAILURE but nothing consumed it. The full flow required the caller to manually call review_rule_failures + brain.patch_rule as the E2E test does. Adds auto_heal_failures(brain, ...) orchestrator in self_healing.py and Brain.auto_heal public method that delegates to it. 6 new tests covering empty input, passing candidate, retro-test failure, in-batch dedup, max_patches cap, and event-log read when no events are passed.
When brain.correct() detects a RULE_FAILURE it now invokes auto_heal_failures inline, capped at one patch per call so a single correction can rewrite at most the rule it just failed. Skipped in dry_run, approval_required, and renter modes; opt-out via correct(auto_heal=False). Adds auto_heal kwarg to brain.correct + brain_correct in _core.py and surfaces the heal summary on event["auto_healed"] when a patch lands. 3 new tests covering the default-on path, the opt-out path, and the dry_run skip. Closes the diagnose-then-do-nothing gap that PR #21 left open.
brain.auto_heal() reads recent RULE_FAILURE events, runs review_rule_failures, gates each candidate through retroactive_test, and applies the survivors via brain.patch_rule. Batches are deduped by (category, original_description) and hard-capped at max_patches (default 5) so a single session can't rewrite a rule multiple times. Before this commit, PR #21 emitted RULE_FAILURE but nothing consumed it. The full flow required the caller to manually call review_rule_failures + brain.patch_rule as the E2E test does. Adds auto_heal_failures(brain, ...) orchestrator in self_healing.py and Brain.auto_heal public method that delegates to it. 6 new tests covering empty input, passing candidate, retro-test failure, in-batch dedup, max_patches cap, and event-log read when no events are passed.
When brain.correct() detects a RULE_FAILURE it now invokes auto_heal_failures inline, capped at one patch per call so a single correction can rewrite at most the rule it just failed. Skipped in dry_run, approval_required, and renter modes; opt-out via correct(auto_heal=False). Adds auto_heal kwarg to brain.correct + brain_correct in _core.py and surfaces the heal summary on event["auto_healed"] when a patch lands. 3 new tests covering the default-on path, the opt-out path, and the dry_run skip. Closes the diagnose-then-do-nothing gap that PR #21 left open.
…() (Phase 1) (#77) * feat(self-healing): close the auto-heal loop brain.auto_heal() reads recent RULE_FAILURE events, runs review_rule_failures, gates each candidate through retroactive_test, and applies the survivors via brain.patch_rule. Batches are deduped by (category, original_description) and hard-capped at max_patches (default 5) so a single session can't rewrite a rule multiple times. Before this commit, PR #21 emitted RULE_FAILURE but nothing consumed it. The full flow required the caller to manually call review_rule_failures + brain.patch_rule as the E2E test does. Adds auto_heal_failures(brain, ...) orchestrator in self_healing.py and Brain.auto_heal public method that delegates to it. 6 new tests covering empty input, passing candidate, retro-test failure, in-batch dedup, max_patches cap, and event-log read when no events are passed. * feat(self-healing): wire auto_heal into brain.correct() by default When brain.correct() detects a RULE_FAILURE it now invokes auto_heal_failures inline, capped at one patch per call so a single correction can rewrite at most the rule it just failed. Skipped in dry_run, approval_required, and renter modes; opt-out via correct(auto_heal=False). Adds auto_heal kwarg to brain.correct + brain_correct in _core.py and surfaces the heal summary on event["auto_healed"] when a patch lands. 3 new tests covering the default-on path, the opt-out path, and the dry_run skip. Closes the diagnose-then-do-nothing gap that PR #21 left open. * fix(self-healing): default auto_heal=False per council; add PatchReceipt for visibility Polyclaude council verdict (4/4 FALSE) on Phase 1: default auto_heal=True is too hot. Ship False default plus a visible PatchReceipt so any auto-heal that does fire is loud, not silent. Flip to True deferred until (a) a pending_patches review queue and (b) ablation showing rule-quality lift on a held-out broken-rule corpus. - brain.correct() and _core.brain_correct() default auto_heal to False - auto_heal_failures() returns PatchReceipt dicts: rule_id, old/new confidence, patch_diff, revert_command (legacy fields kept for BC) - _core emits one stderr line per successful auto-heal patch so silent rule edits cannot sneak through - tests: opt-in explicit auto_heal=True on existing integration tests, plus new test_default_off_no_patch and test_patch_receipt_shape Co-Authored-By: Gradata <noreply@gradata.ai> * fix(self-healing): address CodeRabbit review on #77 (docstring + type ann + test fixture) - Document auto_heal parameter on Brain.correct (defaults to False post-council) - Add forward-ref Brain type annotation on auto_heal_failures via TYPE_CHECKING - Hoist brain_with_rule to module-scope fixture and remove 5 duplicated class-level copies - Split weak test_auto_heal_triggers_on_rule_failure into two deterministic tests: one asserts rule_failure_detected only, the second drives auto_heal_failures with known-good delta words and asserts patched==1 plus auto_heal: reason prefix Co-Authored-By: Gradata <noreply@gradata.ai> * fix(self-healing): address CodeRabbit on inline auto-heal coverage; rebase onto main Rewrite TestSelfHealingE2E::test_inline_auto_heal_emits_rule_patched and TestCorrectAutoHealIntegration::test_auto_heal_emits_patch_when_retroactive_test_passes to exercise ONLY the inline `brain.correct(auto_heal=True)` path. Remove the manual `review_rule_failures()` + `brain.patch_rule()` fallback so a regression in the inline orchestration fails the test instead of being masked by the fallback emitting RULE_PATCHED on its own. Also address three smaller CR items: - `_core.py`: forward `auto_heal` through the cloud branch by falling back to the local pipeline when the flag is set (cloud client does not yet carry the flag), so `auto_heal=True` is no longer silently dropped in cloud-connected brains. - `_core.py`: route the auto-heal notice through `_log.warning` instead of `print(..., file=sys.stderr)` to keep the SDK logging pipeline consistent. - `brain.py`: invalidate `_rule_cache` after `Brain.auto_heal()` applies patches, so later `apply_brain_rules()` calls see the patched prompt. Rebased onto origin/main (includes #76 cloud split, #80 distribution, #81 rule-to-hook auto-promotion). Rebase was conflict-free -- zero file overlap between this branch and the three landed PRs. Co-Authored-By: Gradata <noreply@gradata.ai> * fix(self-healing): use contextlib.suppress for cache invalidation (Ruff SIM105) CodeRabbit's 3rd review on PR #77 + CI Ruff job both flagged the try/except/pass block at brain.py:548-552 as SIM105 violation. Replaced with contextlib.suppress(Exception) (contextlib already imported at line 51). Behavior unchanged. Co-Authored-By: Gradata <noreply@gradata.ai> --------- Co-authored-by: Gradata <noreply@gradata.ai>
Summary
Architecture
Test plan
Generated with Gradata
Greptile Summary
This PR introduces a self-healing engine that closes the feedback loop between rule failures and rule improvement. When a high-confidence
RULEfails to prevent a correction it should cover, the engine detects the failure, emits aRULE_FAILUREevent, generates a deterministic patch candidate, gates it through a retroactive validation step, and (viabrain.patch_rule()) rewrites the rule description with a full audit trail. A nudging subsystem additionally surfaces uncovered correction patterns as pending-approvalINSTINCTlessons. The architecture is well-isolated — every new code path is wrapped in try-except so failures can never break the correction pipeline.Key changes:
self_healing.py: New module withdetect_rule_failure,apply_patch,retroactive_test,review_rule_failures,check_nudge_threshold, andnarrow_rule_scope.brain.py: New publicpatch_rule()API with HMAC re-signing (addresses prior review concern) andRULE_PATCHEDevent emission._core.py: Hooks self-healing detection intobrain_correct()after every correction.rule_canary.py: Replaces the over-broadLIKEquery withjson_extract— but uses the wrong field name$.rule_categoryinstead of$.category, silently disabling rollback detection entirely (see inline comment).rule_integrity.py: Minor cleanup —_ensure_tableswitched to a context-manager pattern.Confidence Score: 3/5
Not safe to merge until the canary field name bug is fixed; the self-healing engine itself is solid, but the rule_canary regression silently disables rollback protection for all graduated rules.
The core self-healing pipeline is well-implemented and the prior re-signing concern is correctly addressed. However, the rule_canary.py fix introduces a silent regression: json_extract(data_json, '$.rule_category') never matches a CORRECTION event (the field is '$.category'), so correction_count is permanently 0 and no canary rule can ever be rolled back. This breaks a protective production safety mechanism. One targeted fix to the field name would bring this to 4/5.
src/gradata/enhancements/rule_canary.py — wrong JSON field name disables canary rollback detection
Important Files Changed
$.rule_categorybut CORRECTION events store the category under$.category; correction_count will always be 0, silently disabling canary rollback detection._generate_deterministic_patchuses non-deterministic set iteration that can produce different patches for identical input across runs.patch_rule()public API with audit trail, re-signing of HMAC after patch, and RULE_PATCHED event emission. Correctly addresses the re-signing concern from a prior review.brain_correct(). Properly guarded by try-except, preserving correction pipeline safety._ensure_tableswitched to context manager form, docstring clarification added for deduplication behaviour. No issues.Sequence Diagram
sequenceDiagram participant U as User participant BC as brain.correct() participant SH as self_healing participant EB as EventBus participant PR as brain.patch_rule() participant RI as rule_integrity U->>BC: correct(draft, final, category) BC->>SH: detect_rule_failure(lessons, category, desc) alt Rule covers category (conf >= 0.80) SH-->>BC: failure dict BC->>EB: emit(RULE_FAILURE, failure) else No covering rule SH-->>BC: None end Note over EB: Background review fork EB->>SH: review_rule_failures(events) SH->>SH: _generate_deterministic_patch() SH->>SH: retroactive_test(original, proposed, correction) alt Delta score >= 0.20 SH-->>EB: patch candidate (passes=True) EB->>PR: patch_rule(category, old_desc, new_desc) PR->>RI: sign_and_store(db_path, new_desc, category) PR->>EB: emit(RULE_PATCHED) else Score too low SH-->>EB: patch candidate (passes=False) endComments Outside Diff (1)
tests/test_pipeline_e2e.py, line 23 (link)The default value
"C:/Users/olive/SpritesWork/brain/cloud-only"is a hardcoded absolute path to a specific developer's machine. On any CI runner, other developer's machine, or non-Windows OS this path won't exist,sys.path.insert(0, _cloud_path)will silently add a non-existent path, and the subsequent import will fall into theexcept ImportErrorbranch — which happens to be the intended behavior, but only by accident.The correct approach is to use
""as the default and guard the path insertion:Rule Used: # Code Review Rules
Rule 1: Never use print() ... (source)
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "merge: resolve conflicts with main after..." | Re-trigger Greptile